Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update dependencies to avoid conflict in WP 6.6 #3181

Merged
merged 47 commits into from
Aug 2, 2024

Conversation

dkoo
Copy link
Contributor

@dkoo dkoo commented Jun 13, 2024

All Submissions:

Changes proposed in this Pull Request:

Refactoring required for changes in Automattic/newspack-scripts#209. Note that this PR will not pass CI tests and should NOT be merged until Automattic/newspack-scripts#209 is released as an NPM package, so this repo can install it as a dependency.

Note: this PR also renames the assets folder to src to bring this repo in line with all other Newspack repos' directory structure.

How to test the changes in this Pull Request:

Follow instructions in Automattic/newspack-scripts#209.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@dkoo dkoo force-pushed the chore/update-dependencies branch from af05b67 to f6b1ed5 Compare June 13, 2024 22:46
@dkoo dkoo self-assigned this Jun 14, 2024
@dkoo dkoo added [Status] Needs Review The issue or pull request needs to be reviewed Do Not Merge! dependencies Pull requests that update a dependency file labels Jun 14, 2024
@dkoo dkoo marked this pull request as ready for review June 14, 2024 23:00
@dkoo dkoo requested a review from a team as a code owner June 14, 2024 23:00
Copy link
Member

@adekbadek adekbadek left a comment

Choose a reason for hiding this comment

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

The only issue I've found is that the linting and typescript checking is broken with these changes. Are these proxy scripts necessary?

package.json Outdated Show resolved Hide resolved
src/components/src/image-upload/index.test.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
@dkoo dkoo requested a review from adekbadek June 25, 2024 21:54
Copy link
Member

@miguelpeixe miguelpeixe left a comment

Choose a reason for hiding this comment

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

No major issues while testing, the only problem I found is what appears to be a conflict between the auto js formatter and the typescript check, which is probably what @adekbadek also identified in his first review.

npm run typescript:check passes with the current formatting, but after the changes from npm run format:js I get a bunch of errors like this on the ts check:

src/wizards/engagement/components/prompt.tsx:245:20 - error TS2339: Property 'indexOf' does not exist on type 'string | number | boolean | number[] | string[]'.
  Property 'indexOf' does not exist on type 'number'.

245                 ]?.indexOf(
                       ~~~~~~~

I don't think it's a blocking issue for now but we'll need to figure out how to adapt the rules so we don't break ts once we commit the rules from the auto formatter.

@dkoo
Copy link
Contributor Author

dkoo commented Aug 1, 2024

@miguelpeixe I'd say the conflict between formatting and TS rules is non-blocking for now, as I don't think we should reformat our JS at this point because the newer rules in @wordpress/scripts would result in a huge number of changes in our JS across all repos. If we decide to change our JS formatting to match, we should do so in future PRs and separately from these other changes, and then we can resolve the conflict with our TS rules.

@dkoo dkoo removed the Do Not Merge! label Aug 2, 2024
@dkoo dkoo merged commit 0d35ac8 into trunk Aug 2, 2024
8 checks passed
@dkoo dkoo deleted the chore/update-dependencies branch August 2, 2024 20:32
matticbot pushed a commit that referenced this pull request Aug 2, 2024
# [5.0.0-alpha.1](v4.8.0-alpha.3...v5.0.0-alpha.1) (2024-08-02)

### Bug Fixes

* **data-events:** ensure get_last_successful_order returns WC_Order ([222f58c](222f58c))
* update dependencies to support `@wordpress/scripts` ([#3181](#3181)) ([0d35ac8](0d35ac8))

### Features

* rename guest contributor role ([#3302](#3302)) ([c7f6917](c7f6917))

### BREAKING CHANGES

* Updates dependencies for compatibility with WordPress 6.6.*, but breaks JS in WordPress 6.5.* and below. If you need support for WP 6.5.*, please do not upgrade to this new major version.

* chore: refactor for updating dependencies in newspack-scripts

* chore: JS formatting fixes for eslint errors

* chore: update NPM scripts and other dependencies

* chore: fix jest unit tests

* chore: update dependencies in newspack-components

* fix: peer dependency conflicts with @wordp
ress/* packages

* chore: update newspack-components dependencies

* chore: proxy stylelint from newspack-scripts

* chore: remove unnecessary stylelint

* refactor: use proxy script for eslint and stylelint scripts

* chore: remove console.log

* chore: update newspack-scripts to NPM alpha version

* chore: update newspack-scripts to git branch

* chore: update newspack-scripts to alpha.3

* test: remove unneeded mocks

* deps: newspack-scripts@5.6.0-alpha.1

* chore: fix scripts

* fix: update newspack-scripts to v5.6.0-alpha.2

* fix: add newspack-revisions.js to webpack build for linting

* fix: update stylelint and format CSS for errors

* chore: clear TS check errors

* fix: start command

* fix: remove i18n script (Blocks only)

* temporarily revert stylelint autoformat to avoid merge conflicts

* chore: add .stylelintrc.js

* fix: reformat SCSS

* chore: update newspack-scripts to v5.6.0-alpha.4

* fix: add missing Prettier config files

* chore: update newspack-scripts to 5.6.0-alpha.5

* chore: update newspack-scripts to v5.6.0-alpha.7

* style: stylelint autofixes

* fix: add fix:js script; temporarily remove format:js

* fix: update newspack-scripts to v5.5.0-alpha.8 and restore format:js script

* chore: add NPM scripts for PHP

* refactor: move recaptcha JS to src directory

* fix: ignore dist and node_modules inside src/components, too

* fix: update Babel configs for components

* fix(components): output TS and JS, and publish new major version to NPM

* fix: do not delete node_modules after post-publish

* chore: bump newspack-scripts to v5.5.2
@matticbot
Copy link
Contributor

🎉 This PR is included in version 5.0.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Aug 13, 2024
# [5.0.0](v4.7.0...v5.0.0) (2024-08-13)

### Bug Fixes

* **data-events:** ensure get_last_successful_order returns WC_Order ([222f58c](222f58c))
* **data-events:** ensure get_last_successful_order returns WC_Order ([#3299](#3299)) ([da21561](da21561))
* **data-events:** race condition between `order_completed` and `subscription_updated` ([#3314](#3314)) ([afbf2e8](afbf2e8))
* force active membership if the user has active subs ([#3268](#3268)) ([aebe581](aebe581))
* namespacing issue ([7b1506a](7b1506a))
* update dependencies to support `@wordpress/scripts` ([#3181](#3181)) ([0d35ac8](0d35ac8))
* **woocommerce:** prevent /shop redirect from author archives ([46db669](46db669))

### Features

* enhancements for the Non editing contributor role ([#3277](#3277)) ([e47e422](e47e422))
* **ga4:** filter custom params ([eda5553](eda5553))
* rename guest contributor role ([#3302](#3302)) ([76ac05e](76ac05e))
* rename guest contributor role ([#3302](#3302)) ([c7f6917](c7f6917))
* show deprecation warning for CM in Engagement wizard ([#3264](#3264)) ([0813369](0813369))

### BREAKING CHANGES

* Updates dependencies for compatibility with WordPress 6.6.*, but breaks JS in WordPress 6.5.* and below. If you need support for WP 6.5.*, please do not upgrade to this new major version.

* chore: refactor for updating dependencies in newspack-scripts

* chore: JS formatting fixes for eslint errors

* chore: update NPM scripts and other dependencies

* chore: fix jest unit tests

* chore: update dependencies in newspack-components

* fix: peer dependency conflicts with @wordp
ress/* packages

* chore: update newspack-components dependencies

* chore: proxy stylelint from newspack-scripts

* chore: remove unnecessary stylelint

* refactor: use proxy script for eslint and stylelint scripts

* chore: remove console.log

* chore: update newspack-scripts to NPM alpha version

* chore: update newspack-scripts to git branch

* chore: update newspack-scripts to alpha.3

* test: remove unneeded mocks

* deps: newspack-scripts@5.6.0-alpha.1

* chore: fix scripts

* fix: update newspack-scripts to v5.6.0-alpha.2

* fix: add newspack-revisions.js to webpack build for linting

* fix: update stylelint and format CSS for errors

* chore: clear TS check errors

* fix: start command

* fix: remove i18n script (Blocks only)

* temporarily revert stylelint autoformat to avoid merge conflicts

* chore: add .stylelintrc.js

* fix: reformat SCSS

* chore: update newspack-scripts to v5.6.0-alpha.4

* fix: add missing Prettier config files

* chore: update newspack-scripts to 5.6.0-alpha.5

* chore: update newspack-scripts to v5.6.0-alpha.7

* style: stylelint autofixes

* fix: add fix:js script; temporarily remove format:js

* fix: update newspack-scripts to v5.5.0-alpha.8 and restore format:js script

* chore: add NPM scripts for PHP

* refactor: move recaptcha JS to src directory

* fix: ignore dist and node_modules inside src/components, too

* fix: update Babel configs for components

* fix(components): output TS and JS, and publish new major version to NPM

* fix: do not delete node_modules after post-publish

* chore: bump newspack-scripts to v5.5.2
@matticbot
Copy link
Contributor

🎉 This PR is included in version 5.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file released on @alpha released [Status] Needs Review The issue or pull request needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants