Update Jetpack ref

[etoledom]

gutenberg side PR: WordPress/gutenberg#26200

This PR updates the reference to the Jetpack Submodule to incorporates the changes from Automattic/jetpack#17123 into gutenberg-mobile.

This PR partially replaces #2616, due to the (temporal) impossibility of updating the original forked branch directly.

To test:

• Run the project.
• Check that the changes from Contact Info Block: changes for adaptation into a mobile block Automattic/jetpack#17123 are visible.
• Smoke-test for possible issues due to updating Jetpack.

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.

[peril-wordpress-mobile]

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

[mzorz]

Smoke tested the feature as per the description and comments in Automattic/jetpack#17123 and found it to be working well with the caveats explained below. 👍

1. We were discussing on the side that there's one thing that prevents the bundler from running which is as follows:

BUNDLE  [android, dev] ./index.js ▓▓▓░░░░░░░░░░░░░ 19.1% (655/1500)::1 - - [16/Oct/2020:10:40:45 +0000] "GET /index.bundle?platform=android&dev=true&minify=false HTTP/1.1" 500 - "http://localhost:8081/debugger-ui/debuggerWorker.d9da4ed7.js" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.4240.80 Safari/537.36"
error: bundling failed: jetpack/extensions/shared/icons.scss: Undefined variable: "$dark-gray-500".

Defining that variable locally for gutenberg-mobile will make it run (see an example of a similar situation here).
@etoledom you found out the problem seems to be only with the metro bundler, right?

It seems there may be a clash in definitions from gutenberg and jetpack for the base-styles/_colors.native.scss brought by @wordpress/base-styles, but consisting these values I think exceeds the purpose of this PR so, I'd say we should be fine defining that specific value as done here should be enough for now. We can open an issue to continue discussion elsewhere. Let me know what you think about this.

2. The other issue is that we're ignoring the node version in order to surpass the problem first observed in this other PR Added yarn install on jetpack folder #2728 in CI here, which sounds OK for the time being but we should probably try and address this differently, also on a different PR and to be discussed elsewhere. I worry that having the ignore flag for long can make some node engine update forgotten inadvertedly or some other unexepected issues may arise.

With knowing these, again I'd say we should be good to go with this one and open other issues elsewhere to keep the discussion going, wdyt @etoledom ?

[etoledom]

Thank you for your insight @mzorz !

I'd say we should be fine defining that specific value as done here should be enough for now. We can open an issue to continue discussion elsewhere. Let me know what you think about this.

I agree with this, let's solve the issues to merge this PR, then we can think about a proper solution for all these problems.
For sure we will keep having problems like this while we add more Jetpack Blocks.

cc @maxme @hypest

I have added the missing color to gutenberg/packages/base-styles/_colors.native.scss.

Other alternatives I tried (and failed):

• Adding jetpack/node_modules/... to Metro's extraNodeModules config property.
  • Since the color is properly defined on jetpack/node_modules/@wordpress/base-styles/_colors.native.scss.
  • Also, since CI was passing previously, this seems to be a Metro-(dev)-only issue.
• Defining this color on gutenberg-mobile side.
  • Since it's a jetpack related issue. Needed to extend sass-transformer.js from gutenberg-mobile root to add extra paths to be considered by the transformer. Failed to do this on a short spike.

[maxme]

Defining this color on gutenberg-mobile side.

* Since it's a `jetpack` related issue. Needed to extend `sass-transformer.js` from `gutenberg-mobile` root to add extra paths to be considered by the transformer. Failed to do this on a short spike.

IMO this is the ideal solution. The conflict appears at this level, and it should be fixed at this level. Having said that, the fix on the core side is quite simple and don't affect the rest of the project (this could only become an issue if there are linters that checks for unused resources).

[mzorz]

Just merged WordPress/gutenberg#26200, would you like to update gutenberg's hash here so we can merge afterwards @etoledom ?

[mzorz]

LGTM

[etoledom]

Thank you all! 🙏