Skip to content

WIP: Update wordpress monorepo#17072

Closed
brbrr wants to merge 4 commits intomasterfrom
try/update-wordpress-monorepo
Closed

WIP: Update wordpress monorepo#17072
brbrr wants to merge 4 commits intomasterfrom
try/update-wordpress-monorepo

Conversation

@brbrr
Copy link
Copy Markdown
Contributor

@brbrr brbrr commented Sep 4, 2020

Fixes #

Changes proposed in this Pull Request:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

  • Go to '..'

Proposed changelog entry for your changes:

@matticbot
Copy link
Copy Markdown
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello brbrr! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D49093-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Sep 4, 2020

Fails
🚫

node failed.

Log

Error:  { Error: [BABEL] /home/travis/build/Automattic/jetpack/dangerfile.js: Cannot find module '@babel/compat-data/plugin-bugfixes'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
    at Function.Module._load (internal/modules/cjs/loader.js:562:25)
    at Function._module2.default._load (/home/travis/build/Automattic/jetpack/node_modules/override-require/dist/overrideRequire.js:43:25)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (/home/travis/build/Automattic/jetpack/node_modules/@automattic/calypso-build/node_modules/@babel/preset-env/lib/plugins-compat-data.js:10:46)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.customModuleHandler (/home/travis/build/Automattic/jetpack/node_modules/danger/distribution/runner/runners/inline.js:116:28)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12) code: 'MODULE_NOT_FOUND' }
danger-results://tmp/danger-results.json

Generated by 🚫 dangerJS against 3c18e96

path.join( __dirname, './extensions/shared/i18n-to-php' )
),
new webpack.NormalModuleReplacementPlugin(
/^@wordpress\/element$/,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I had one more idea.

Having NormalModuleReplacementPlugin replace imports from @wordpress/element has proven tricky, since our replacement -- element-to-php.js -- would need to include all export symbols from that package (with only createInterpolateElement replaced with the identity). Unlike the counterpart for @wordpress/i18n, @wordpress/element contains a plethora of export symbols, and the mix of wildcard re-exports that we've so far tried in this PR hasn't worked; and listing them all manually would be a maintenance nightmare, and very fragile.

But the NormalModuleReplacementPlugin should also work at a deeper level, so we might try to replace the ./create-interpolate-element import at package root level instead:

Suggested change
/^@wordpress\/element$/,
/^\.\/create-interpolate-element$/,

(if my RegEx-fu isn't mistaken)

However, this means that we'll also need to stop externalising @wordpress/element, and include it in our build instead.

cc/ @brbrr @anomiex @jeherve

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like that might actually work (without needing to touch the externalised stuff?) Pushed a commit to #16763 (as deps are more up-to-date there). Let's continue there.

@ockham
Copy link
Copy Markdown
Contributor

ockham commented Dec 1, 2020

(This PR also needs a rebase, to include e.g. #17935. Also, the package versions in here are probably outdated now -- it might make sense to carry them over from #16763.)

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.

4 participants