Automated Testing: Enforce no-unresolved checks for test files

[aduth]

What?

Updates ESLint configuration to enforce import/no-unresolved rules consistently, regardless of "development files" status. Includes fixes for existing issues.

Builds upon #79703. The earlier pull request focused on import/no-extraneous-dependencies, adding missing dependencies to package.json. This one focuses on import/no-unresolved and improving the accuracy of our custom resolver in tools/eslint/import-resolver.cjs to account for false negatives.

Why?

As explained in #79703, there's no special reason for these files to be exempt from these rules that are meant to help us ensure that imports are valid. The reasons for disabling them are likely a combination of (a) temporarily ignoring failing files that were deemed "less critical" and (b) limitations of the default ESLint import resolver described in #72136 which have since been resolved.

How?

• Removed import/no-unresolved override (exemption) from ESLint configuration
• Address existing issues, most of which are related to the custom import resolver behavior

We use a custom import resolver to remap import requests to their source files, so that running ESLint doesn't require a full build to happen before it's run. Previously, this custom resolver was rather naive about handling exports entrypoints, but the naivety was largely unsurfaced because of the previous exemptions. Removing these exemptions revealed the shortcomings, which are addressed in the changes here.

Notably, it now supports wildcard path matching (needed for e.g. block-library block exports), resolving exports which are mapped directly to a string (as opposed to an object of import, default, types, etc.), and resolving more file extensions (.jsx and .cjs). Ideally, the exports resolution wouldn't be something we implement ourselves. Packages like the popular resolve have options for providing a package object, but it still does filesystem operations that make it heavier than we need it to be. Its internal resolveExports helper would be handy and is in many ways what we've implemented here, but it isn't exposed as part of the library's public API. I'd be open to considering another library which is expressly aimed at resolving package.json exports if one exists.

Testing Instructions

1. Run npm run clean:packages to delete any built artifacts to effectively exercise the expected resolver behavior that it doesn't rely on built package contents to lint
2. npm run lint:js should pass.

Use of AI Tools

I used Cursor IDE + Composer model to do an assessed categorization of the issues reported after removing import/no-unresolved. But the actual code changes were largely manual, with a bit of tab completion here and there.

[github-actions]

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: aduth <aduth@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: manzoorwanijk <manzoorwanijk@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[aduth]

I don't know that this was working at all before for anything other than the . entrypoint. With this previous logic, subpath would end up would end up like build-module/paragraph/index.mjs (for the @wordpress/block-library example in the changes above) because path.join only emits a leading dot if the given path segments are empty. But each exports key must have a leading dot:

All target paths in the "exports" map (the values associated with export keys) must be relative URL strings starting with ./.

https://nodejs.org/api/packages.html#targets-must-be-relative-urls

So trying to access manifest.exports?.[ 'build-module/paragraph/index.mjs' ] would never work (ignoring the fact the package defines the matching export with a wildcard).

[manzoorwanijk]

I will test this for isolated mode tomorrow.

[github-actions]

Size Change: -37 B (0%)

Total Size: 7.61 MB

📦 View Changed

Filename	Size	Change
build/modules/vips/worker.min.js	3.69 MB	-37 B (0%)

_{compressed-size-action}

[aduth]

These changes break the unit tests, which similarly expect to be able to run without a preceding build. Which leaves us in a tricky spot because the point of the import rules is to ensure valid imports, and the previous code here is not valid: @wordpress/block-library doesn't export /src/ on its package API surface.

Not entirely sure yet what the best option here would be, but a couple ideas to explore:

• Making block implementations available on the root package exports (affects the public API in a way I'm not sure we're comfortable with)
• Mirroring the same kind of remapping resolver logic we're implementing here for ESLint but in Jest

[aduth]

• Mirroring the same kind of remapping resolver logic we're implementing here for ESLint but in Jest

We already have something like this, but only supporting top-level package imports:

gutenberg/test/unit/jest.config.js

Lines 25 to 31 in 0fc4838

	// Finds all packages which are transpiled with Babel to force Jest to use their source code.
	const transpiledPackageNames = glob(
	path.join( ROOT_DIR, 'packages/*/src/index.{js,ts,tsx}' )
	).map( ( fileName ) => {
	const relative = path.relative( ROOT_DIR, fileName );
	return relative.split( path.sep )[ 1 ];
	} );

gutenberg/test/unit/jest.config.js

Lines 47 to 48 in 0fc4838

	[ `@wordpress\\/(${ transpiledPackageNames.join( '|' ) })$` ]:
	'packages/$1/src',

It looks like we already had to hack around this for some package subpath exports, like in @wordpress/theme:

gutenberg/test/unit/jest.config.js

Lines 49 to 50 in 0fc4838

	'@wordpress/theme/design-tokens.js':
	'<rootDir>/packages/theme/src/prebuilt/js/design-tokens.mjs',

Maybe that's the short-term option here: Add a remapping for these block imports to simulate support, and separately work to mirror the remapping behavior consistently between ESLint and Jest.

[aduth]

The previous syntax here was technically valid for exposing an entire directory and isn't supported in the custom resolver changes introduced with this pull request. But this way of defining exports is deprecated and I think we should move away from it entirely.

[ciampo]

It looks like there’s one more generated-artifact case to handle before the global import/no-unresolved exemption can be removed.

CI is still failing on:

packages/block-serialization-spec-parser/test/index.js:9:23
Unable to resolve path to module '../'. (import/no-unresolved)

This one seems separate from the discussion above.

Here ../ resolves through @wordpress/block-serialization-spec-parser’s package root to parser.js, which is generated by npm run build:js and is not present in a clean checkout.

We can either add a well-documented lint exception, or somehow teach the resolver about this specific generated entrypoint? Probably better to avoid broadening the development-file exemption again, since that would undercut the main goal of this PR.

[aduth]

Hm, yeah, that's an interesting one. Maybe it's reasonable to prebuild this specific artifact in the same way we already do for @wordpress/icons and @wordpress/theme via prelint:js. It starts to chip away from the intent of lint not requiring a pre-build and makes the lint itself a bit heavier, but it's consistent with what we've already been doing. An alternative would be to store those prebuilt parsers in source control, similar to what we do for most theme files.

[aduth]

The prelint:js step seems like it'll be the best short-term option, implemented in 9c64d86. The parser's build is quite fast (<0.5s even with the npm overhead), so not too concerning to have as a pre-step for lint:js. I do think this is something we should try to move away from in favor of files which are statically available in source control. Separately I was looking at if we could get rid of the theme package prebuild, and it might be possible if we make a shift to ESM-only for the package (related comment at #75589 (comment)).

[ciampo]

Agreed, a prelint build weakens the PR’s “lint without build artifacts” ideal, but it is consistent with existing icons/theme precedent and fast.

Also agree that the long-term solution is to make any artifacts statically available