Skip to content

Fix Gutenberg Mobile dependencies resolution#3659

Merged
fluiddot merged 3 commits into
developfrom
fix/metro-extra-modules-gb-deps
Jun 24, 2021
Merged

Fix Gutenberg Mobile dependencies resolution#3659
fluiddot merged 3 commits into
developfrom
fix/metro-extra-modules-gb-deps

Conversation

@fluiddot
Copy link
Copy Markdown
Contributor

@fluiddot fluiddot commented Jun 24, 2021

I added a new dependency to the package.json file of Gutenberg Mobile in this PR and I noticed that Metro was throwing an error due to not being able to resolve it.

After checking the extra node modules resolver, I realized that the dependencies that are not resolved via the Gutenberg submodule were considered to be located in the Jetpack submodule, including the ones defined in Gutenberg Mobile. To address this, I basically added a condition that checks if the path exists before considering it a dependency within Jetpack.

Additionally, I removed the dependency email-validator from Gutenberg Mobile because it's already in the Jetpack submodule, in fact, it's not even referenced in the Gutenberg Mobile code.

As a side note, these are the dependencies resolved from the Jetpack submodule:

  • @automattic/color-studio
  • email-validator

To test:

  • CI jobs should be green on this PR
  • Demo app should run normally
  • As a double-check, verify that the Contact info block works as expected because it's the one that imports the email-validator package.

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.

fluiddot added 2 commits June 24, 2021 12:53
Gutenberg mobile dependencies were resolved using the Jetpack path, so now we check first if the module exists in the Jetpack submodule before solving it with that path.
This dependency is already defined in Jetpack submodule and it's only used there.
@fluiddot fluiddot added the [Type] Bug Something isn't working label Jun 24, 2021
@fluiddot fluiddot self-assigned this Jun 24, 2021
@fluiddot fluiddot requested a review from hypest June 24, 2021 11:32
@fluiddot fluiddot mentioned this pull request Jun 24, 2021
2 tasks
@fluiddot fluiddot changed the title Fix Gutenberg Mobile dependency resolver Fix Gutenberg Mobile dependencies resolution Jun 24, 2021
Comment thread metro.config.js Outdated
@peril-wordpress-mobile
Copy link
Copy Markdown

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

Copy link
Copy Markdown
Contributor

@hypest hypest left a comment

Choose a reason for hiding this comment

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

LGTM! Let's auto-merge when CI green.

✅ CI jobs should be green on this PR. It was green on 0837df7 already.
✅ Demo app should run normally
✅ As a double-check, verify that the Contact info block works as expected because it's the one that imports the email-validator package. ➡️ I verified that when the email is well-formed, the html output includes a mailto: link, while if not well-formed it's just plain text, just like expected by the code.

@fluiddot fluiddot enabled auto-merge June 24, 2021 13:40
@fluiddot fluiddot merged commit a924de0 into develop Jun 24, 2021
@fluiddot fluiddot deleted the fix/metro-extra-modules-gb-deps branch June 24, 2021 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants