Testing: Remove custom import resolvers and package subpath syntax rules #72978
+59
−47
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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. |
aduth
added
the
[Type] Automated Testing
Member
Author
|
Evidenced by the build, one downside of this is that it requires an upfront |
|
Size Change: 0 B Total Size: 2.54 MB ℹ️ View Unchanged
|
|
Flaky tests detected in 9dc01ec. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19649959414
|
Member
Author
Possible solutions:
|
aduth
force-pushed
the
remove/eslint-custom-resolve
branch
from
ee33892 to
a6ff3de
Compare
Member
Author
In a6ff3de, I ended up with a hybrid approach reintroducing the custom resolver, but enhancing it with support to consider subpaths using the package's |
Redundant with improved resolver behavior
Redundant with TypeScript resolver changes
aduth
force-pushed
the
remove/eslint-custom-resolve
branch
from
a6ff3de to
9dc01ec
Compare
Contributor
|
What would be the impact on DevX if we required build before linting? Is it just for CI? If that's the case maybe we should just go for it? |
Member
Author
I think it could be negative enough in regular development workflows to be worth trying to avoid. It might not always crop up because usually a regular contributor will likely have some form of built packages in their local copy of the project, but (a) it could be a barrier for new contributors working from a fresh clone and (b) could create issues where linting identifies issues only because the built copy is out of date. In both cases, these would clear up after running |
youknowriad
approved these changes
Nov 25, 2025
This handles the Babel stuff. It should probably live in eslint-plugin
aduth
commented
Nov 25, 2025
| wp: 'readonly', | ||
| }, | ||
| settings: { | ||
| 'import/internal-regex': wpPackagesRegExp, |
Member
Author
There was a problem hiding this comment.
Noting that this can have an impact on rules like import/order in considering packages as "internal", though (a) this base config exists in a reusable package for third-party consumption where @wordpress/ packages are not internal, and (b) even inside the context of this project, we distinguish @wordpress/ packages from "internal" dependencies.
It might still make sense to define this at the root /.eslintrc.js since, after all, the setting is described as "useful when you are utilizing a monorepo setup or developing a set of packages that depend on each other". But it doesn't seem like something which will actually have much of an impact in practical usage from what I can tell.
Related: #73396
Member
Author
There was a problem hiding this comment.
Noting that this can have an impact on rules like
import/orderin considering packages as "internal", though (a) this base config exists in a reusable package for third-party consumption where@wordpress/packages are not internal, and (b) even inside the context of this project, we distinguish@wordpress/packages from "internal" dependencies.
I added an extra CHANGELOG note clarifying this in 0748af8.
It might still make sense to define this at the root
/.eslintrc.jssince, after all, the setting is described as "useful when you are utilizing a monorepo setup or developing a set of packages that depend on each other". But it doesn't seem like something which will actually have much of an impact in practical usage from what I can tell.
Funny enough, while this might make sense to do, I recalled that one of the changes I'm making in this branch is to remove the code where we explicitly unset this value in our .eslintrc.js. So this should change should have no net effect in this project. Ironic that we were defining this default internal regular expression and not using it in the one place it might make sense to do so 😅
There are a few early returns or thrown (suppressed) errors possible before when subpath is actually used, so avoid wasting effort if it's not going to be used
Member
Author
|
I had some trouble with the tests for
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Updates lint configuration to remove redundant import resolvers and syntax rules.
Follow-up to #72893
Why?
These configurations were previously necessary because:
But these are no longer necessary as of:
exports)exports, which is the standard way to define a package's public interface and disallow subpath importsFurthermore, this change will also allow packages to define their own
exportson subpaths without triggering the "Path access on WordPress dependencies is not allowed" lint error.How?
no-restricted-syntaxselector for path access on packagestools/eslint/import-resolver.jscustom resolver and configured references to itTesting Instructions
npm run lint:jsshould pass.Additionally, you can verify that the original intent of the subpath rules is upheld when trying to import from a subpath of a package: