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

Build: Use regular expressions to mark packages as external #1589

Merged
merged 1 commit into from
Jul 9, 2024

Conversation

adamziel
Copy link
Collaborator

@adamziel adamziel commented Jul 9, 2024

Motivation for the change, related issues

Follows up on #1586

Extracting external packages names from the package.json files did not work as expected due to cwd misconfiguration in CI. This PR replaces that logic with a regexp matching the name of the packages from this monorepo to avoid maintaining the paths entirely.

Testing instructions

Run nx build php-wasm-node and confirm the resultingdist/packages/playground/client/index.js file and other built npm-published modules do not bundle any external modules. The only exception should be the cli packages.

@adamziel adamziel added [Type] Bug An existing feature does not function as intended [Type] Repo / Project Management labels Jul 9, 2024
@adamziel adamziel merged commit a022e4c into trunk Jul 9, 2024
1 check passed
@adamziel adamziel deleted the build-regex branch July 9, 2024 08:19
adamziel added a commit that referenced this pull request Jul 9, 2024
Follows up on
#1589 and #1586

Treating package IDs matching `node_modules` as external works, but
causes Rollup to substitute the module name with the absolute module
path on the host machine, e.g. `import "ini"` becomes `import
"/home/runner/work/wordpress-playground/wordpress-playground/node_modules/ini/lib/ini.js"`
which is not what we want.

This PR sources the external packages names from the package.json file
to both treat all these imports as external AND preserve their original
import identifier.

 ## Testing instructions

Run nx build php-wasm-node and confirm the resultingdist/packages/php-wasm/index.js file and other built npm-published modules do
import external modules using their name, not their absolute path.
adamziel added a commit that referenced this pull request Jul 9, 2024
Follows up on
#1589 and #1586

Treating package IDs matching `node_modules` as external works, but
causes Rollup to substitute the module name with the absolute module
path on the host machine, e.g. `import "ini"` becomes `import
"/home/runner/work/wordpress-playground/wordpress-playground/node_modules/ini/lib/ini.js"`
which is not what we want.

This PR sources the external packages names from the package.json file
to both treat all these imports as external AND preserve their original
import identifier. It also adds the required `package.json` building
steps to the `@php-wasm/universal` package and makes the
`@php-wasm/stream-compression` package public to ensure the published
packages will reference the correct dependencies.

 ## Testing instructions

Run nx build php-wasm-node and confirm the
resultingdist/packages/php-wasm/index.js file and other built
npm-published modules do import external modules using their name, not
their absolute path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended [Type] Repo / Project Management
1 participant