Use package.json name as the script-module ID source of truth#78822
Use package.json name as the script-module ID source of truth#78822retrofox wants to merge 50 commits into
Conversation
parametric 0-9 glyphs with per-stroke gradients, all WPDS tokens
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: 0 B Total Size: 8.21 MB |
|
Following up on the concern @youknowriad raised on #77226:
I'd like to address this here by making the script-module ID compatibility-aware. The ID becomes name plus a key derived from the package's own version: the major for >= 1.0.0, or major.minor for 0.x (where minors can still break). So @org/shared-views at 1.4.2 registers as @org/shared-views@1 and at 2.0.0 as @org/shared-views@2. Plugins on incompatible majors get distinct IDs and never collide; plugins on compatible versions keep the same ID and still dedupe. The npm name, the import specifier, and IDE resolution stay unchanged; the key is a build-time discriminator only on the registered ID and the externalized specifier the build rewrites into consumer bundles. It does not reconcile within a major (first-wins can still pick an older minor), but that is semver-safe and pairs with #77465 to make registration deterministic. One question before I work on it: does keying by compatibility boundary address your concern, or is the objection to separate plugins sharing a registered ID at all, even when versions are compatible? The first is what this targets; the second would be a different shape worth settling first. |
|
If these packages have singletons (like a lot of wordpress packages) using different versions break the behavior regardless of whether they register multiple script versions. What are you trying to solve by registering the same script multiple times? I see only downsides to this and no upsides. I think if the package can exist multiple times, it's probably better to not be a "script" and just be a bundled package. |
|
I think a large part of this problem comes from the fact that "wp-build" currently performs automatic registration for generated script modules/packages. If automatic registration were removed from "wp-build", and developers were responsible for explicitly registering and enqueueing their own modules, this concern would become much smaller. In that model, a shared package would be built once and exposed under a specific module ID. When third-party developers consume that package from multiple plugins, they could intentionally use the same module ID/handle. Since WordPress only needs a single registration for that identifier, the module would effectively be shared rather than duplicated, reducing the likelihood of runtime conflicts caused by automatic registration behavior. The current challenge is that "wp-build" generates registration code automatically. When multiple plugins independently register the same module, questions arise around registration order, version compatibility, and override behavior. This is where most of the complexity discussed in this thread seems to originate. In other words, the shared-package problem may be less about module identity itself and more about the fact that registration is currently automated rather than being left under the control of plugin authors. |
This is one of the main point of wp-build. It's a convention based tool, if you don't like the conventions and you're comfortable with writing glue code yourself, don't use it. |
That is fair, and you're right about the safe default. Thanks for the clarification. I got confused while looking for a solution and finally fell down a classic rabbit hole. I'd like to narrow this PR to the part that stands on its own and has not been contested: using That aligns the npm name, the import specifier, and the registered When the ID is derived from Two things follow from that: anyone can claim the scope and publish under it, so misconfigured tooling could resolve the import to a substituted package; and even with no confused tooling in play, automated scanners flag an imported specifier that is absent from npm, which becomes a steady trickle of (usually bogus) security reports to triage. Deriving the ID from It adds no sharing or extra registration, and the build output is byte-identical for Core (0 B). I will drop the discovery/dedup half so the sharing question can be settled on its own, and I am happy to open that as a separate thread 🙇♂️ |
Narrow the change to name-as-identity: derive the script-module ID from package.json#name and externalize internal-package imports by exact name. Defer cross-plugin sharing/dedup to a separate change.
This is not safe. Semver allows a patch or minor version bump for added functionality that is backwards compatible for code expecting an older version. So, for example, a consumer expecting 1.0.0 should work with 1.4.2, but there's no guarantee at all that a consumer expecting 1.4.2 will work if given 1.0.0 instead. |
Yes, that's correct. I've already removed that part of the PR, @anomiex. |
| } | ||
|
|
||
| // Externalize internal packages by exact name. Must precede the | ||
| // namespace pattern below so internal names win over the wildcard. |
There was a problem hiding this comment.
I don't understand this change, can you explain it please? Why do we need this and why it was not needed before?
There was a problem hiding this comment.
This is the companion to the "name-as-ID" change in this PR; the two are coupled.
Before, the script-module ID was derived from packageNamespace (@<packageNamespace>/<dir>), so every internal package's specifier started with @<packageNamespace>/, and one namespace pattern (^@<packageNamespace>/) externalized all of them.
With "name-as-ID", the ID is the package's own package.json#name, under an owned scope (e.g. @org/feature, not @my-plugin/feature).
The namespace pattern can't simply be pointed at that scope: ^@org/ would match every @org/* import, not just this plugin's packages, so genuine third-party @org/* dependencies would be swept into the externalization rule too.
Internal packages are therefore externalized by their exact names, a precise set that matches only the known packages, whatever their scope.
For Core, it is a no-op: every name already equals @wordpress/<dir>, so the exact-name set and the namespace pattern externalize those imports identically.
The handler does overlap with the namespace one below it (same script-module/script handling and dependency tracking).
I kept this PR limited to identity; as a follow-up, I'd unify them into a single resolution path and add characterization tests that lock the behavior. Happy to open that separately.
c2165eb to
8a40c80
Compare
|
Flaky tests detected in ddc46b0. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/27087991671
|
Problem
wpPlugin.packageNamespacecarries three roles in wp-build today:In Core the three collapse into one identifier because Gutenberg owns the
@wordpressscope and every package's directory name matches its npm name:wp-build, npm, the IDE, and
wp_register_script_moduleall see@wordpress/blocks.In an organization-owned monorepo with multiple plugins, that alignment breaks. Each plugin needs its own
packageNamespace, but every package's npm name has to live under the org-owned scope:wp-build derives the script-module ID (and the externalized specifier) from
packageNamespace, so it emits@plugin-a/featurewhile the npm name is@org/plugin-a-feature. Same package, two strings, and the divergence costs:tsconfigpathsalias to bridge the emitted specifier back to the real package.@plugin-a/is an npm scope nobody owns. Anyone can register it, so misconfigured tooling could resolve the import to a substituted package; and even with no confused tooling in play, dependency-confusion scanners flag a specifier that is imported but absent from npm, which becomes a steady trickle of (usually bogus) security reports to triage.The runtime indirection already exists.
wp_register_script_module( $id, ... )accepts arbitrary IDs and the PHP registry treats them as opaque strings. wp-build is the only piece tying identity to a config-derived shortcut.Proposal
Derive each script module's ID from the package's own
package.json#name. Thewordpress-externalsplugin externalizes internal-package imports by exact name, regardless ofpackageNamespace. The legacy@<packageNamespace>/<dir>shape stays as the fallback for a package without aname.The npm name, the import specifier, and the registered
$idbecome one string end-to-end: notsconfigalias, and no unregistered-scope surface for scanners to flag.Compatibility
No-op for Core. Every Gutenberg package's
namealready matches the legacy derivation, so a full build on this branch produces a byte-identicalbuild/modules/tree compared to trunk (0 B size change).For a consumer with the dual-naming pattern, the migration is renaming the package's
namefield to its owned npm name; the directory does not need to move.Scope
This PR is intentionally limited to identity. Whether wp-build should also discover and register a single shared copy of a cross-plugin package (instead of each consumer bundling its own) is a separate question with its own trade-offs, and it is left to a separate thread so it can be settled on its own. Nothing here adds shared registration or changes how packages are bundled.
Context
Companion: #78715 makes
getPackageInforeturnnullon a resolve miss as its JSDoc already promises; the exact-name handler introduced here relies on that guard for monorepos whosetsconfigpathsaliases resolve outsidenode_modules.Supersedes #77226 (which extended discovery via a
packageSourcesconfig). Was opened as #78810; reopened here so the branch lives onoriginrather than a fork.