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

Update submodule dependencies #4383

Merged
merged 7 commits into from
Dec 17, 2021
Merged

Conversation

dcalhoun
Copy link
Member

@dcalhoun dcalhoun commented Dec 16, 2021

Update Gutenberg version and capture lock file changes originating from dependency updates within the gutenberg submodule.

Additionally, the Jest test runner was resolving different version of Jest sub-dependencies, which resulted in the following error.

TypeError: snapshotResolver.resolveSnapshotPath is not a function

  at Object._default [as default] (node_modules/jest-jasmine2/build/setup_jest_globals.js:101:41)

Specifically, the test runner resolved node_modules/[email protected] > gutenberg/node_modules/[email protected]. jest-snapshot@27 includes a breaking change of a newly returned Promise: jestjs/jest#8829

This reordering prioritizes the dependencies found in the top-level node_modules directory over the directory found within the gutenberg submodule. The origin of this incorrect module resolution is a combination of WordPress/gutenberg#37396 and WordPress/gutenberg#33448.

To test: Passing CI checks should suffice.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes more info and have added them to RELEASE-NOTES.txt if necessary.

@dcalhoun dcalhoun marked this pull request as draft December 16, 2021 15:32
These changes originate from updated dependencies within the `gutenberg`
submodule.
@dcalhoun dcalhoun force-pushed the chore/update-submodule-dependencies branch from b12bad1 to 1afcc85 Compare December 16, 2021 15:38
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Dec 16, 2021

Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job!

The outdated packages resulted in the following CI failure.

```
TypeError: snapshotResolver.resolveSnapshotPath is not a function

  at Object._default [as default] (node_modules/jest-jasmine2/build/setup_jest_globals.js:101:41)
```

These updates were completed to resolve the error and mirror updates
within the `gutenberg` submodule, which updated `jest-snapshot`:
WordPress/gutenberg#37396

The cause of the error is a breaking change that landed in
`jest-snapshots@27`: jestjs/jest#8829
jest.config.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@dcalhoun dcalhoun marked this pull request as ready for review December 16, 2021 17:06
@dcalhoun dcalhoun enabled auto-merge December 16, 2021 17:14
@dcalhoun dcalhoun self-assigned this Dec 16, 2021
@dcalhoun dcalhoun disabled auto-merge December 16, 2021 19:59
@dcalhoun dcalhoun removed their assignment Dec 16, 2021
fluiddot
fluiddot previously approved these changes Dec 17, 2021
Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

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

I've just added a couple of minor comments, the PR LGTM 🎊 !

  • Confirmed that CI checks are passing ✅
  • Run npm install locally and verify that package-lock.json is not modified ✅

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
The Jest test runner found was resolving different version of Jest
sub-dependencies, which resulted in the following error.

```
TypeError: snapshotResolver.resolveSnapshotPath is not a function

  at Object._default [as default] (node_modules/jest-jasmine2/build/setup_jest_globals.js:101:41)
```

Specifically, the test runner resolved
`node_modules/[email protected] >
gutenberg/node_modules/[email protected]`. `jest-snapshot@27`
includes a breaking change of a newly returned `Promise`:

jestjs/jest#8829

This reordering prioritizes the dependencies found in the top-level
`node_modules` directory over the directory found within the `gutenberg`
submodule.
@dcalhoun dcalhoun requested review from guarani and fluiddot December 17, 2021 17:23
@dcalhoun dcalhoun self-assigned this Dec 17, 2021
These changes originate from updated dependencies within the `gutenberg`
submodule. These changes are the result of clearing out all
`node_modules` directories, and running `npm install` three times.
@dcalhoun dcalhoun enabled auto-merge December 17, 2021 18:26
@dcalhoun dcalhoun dismissed fluiddot’s stale review December 17, 2021 18:27

The proposed changes have changed drastically since review.

@dcalhoun dcalhoun disabled auto-merge December 17, 2021 18:30
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

The changes look good!
In the package-lock.json file I see only changes to @types/react and @types/react-dom which come from the update to the Gutenberg submodule (I see the same version updates there).
Thanks so much @dcalhoun!

@dcalhoun dcalhoun merged commit abb81d3 into develop Dec 17, 2021
@dcalhoun dcalhoun deleted the chore/update-submodule-dependencies branch December 17, 2021 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants