Skip to content

Conversation

@kienstra
Copy link
Contributor

@kienstra kienstra commented Jul 24, 2023

Before

Every time I updated npm packages, that caused errors with Jest unit tests or npm run build.

And composer install failed on PHP 8.2, as the packages weren't compatible with 8.2.

After

The npm package versions don't matter much anymore.

And it'll be easier to update package versions, without a Jest or PHPUnit error.

Changes

  • Remove React component unit tests. Testing the markup layer was a bad idea (that I had). It meant that every time we updated the npm packages, Jest tests could fail, like from React versions being out of sync.
  • Remove --webpack-no-externals. This should make npm package updates less necessary. The block editor will alias them to whatever packages it's using.
  • Update @wordpress/* dependencies
  • Make unit and e2e tests pass

Testing instructions

Not needed

@kienstra kienstra changed the title Get out of dependency hell Get out of dependency 'heck' Jul 24, 2023
@kienstra kienstra mentioned this pull request Jul 24, 2023
"plugin:import/recommended",
"plugin:eslint-comments/recommended",
"plugin:jsx-a11y/recommended",
"plugin:jest-dom/recommended",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was for Jest unit tests of React components, which I deleted

"license": "GPL-2.0",
"require": {
"php": "^5.6 || ^7 || ^8"
"php": "^7 || ^8"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core 6.3 doesn't support PHP 5.6 anymore.

That's a good excuse to remove our support, as it's really hard to make composer install work for PHP 5.6-8.2.


const testImagePath = path.join( __dirname, '..', 'assets', fileName );
const newFileName = uuid();
const newFileName = Math.floor( Math.random( 100000 ) * 100000 );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to import uuid, we can do this in 1 line of JS.

@kienstra
Copy link
Contributor Author

Hi @dreamwhisper,
Thanks a lot for reviewing this!

Ease of release: Replace `gulp` with `wp-svn` orb
@kienstra kienstra merged commit 5562fad into develop Jul 27, 2023
@kienstra kienstra deleted the fix/dependency-hell branch July 27, 2023 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants