close
Skip to content

wp-build: Replace import.meta.resolve with createRequire in getPackageInfo#74171

Closed
lezama wants to merge 1 commit intoWordPress:trunkfrom
lezama:fix/wp-build-pnpm-compatibility
Closed

wp-build: Replace import.meta.resolve with createRequire in getPackageInfo#74171
lezama wants to merge 1 commit intoWordPress:trunkfrom
lezama:fix/wp-build-pnpm-compatibility

Conversation

@lezama
Copy link
Contributor

@lezama lezama commented Dec 22, 2025

Refactors package resolution in getPackageInfo to use Node's createRequire and require.resolve instead of import.meta.resolve and fileURLToPath. This improves compatibility and reliability when resolving package.json files.

What?

Closes

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

Before After

@github-actions
Copy link

github-actions bot commented Dec 22, 2025

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: lezama <migueluy@git.wordpress.org>
Co-authored-by: SirLouen <sirlouen@git.wordpress.org>
Co-authored-by: youknowriad <youknowriad@git.wordpress.org>

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

@lezama lezama force-pushed the fix/wp-build-pnpm-compatibility branch from cfb88cd to 62f230e Compare December 22, 2025 18:53
Copy link
Member

@SirLouen SirLouen left a comment

Choose a reason for hiding this comment

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

It's seems that the original code prevented a second resolution. of the same package name with the packageJsonCache. I'm not 100% aware of how require.resolve works behind the scenes and maybe it's not worth, but I wonder if it would be a good idea to also implement some sort of cache something equivalent to this declaration, const packageJsonCache = new Map(); but adapted to this require.resolve version.

@SirLouen SirLouen requested a review from youknowriad December 23, 2025 21:57
@SirLouen SirLouen added [Type] Enhancement A suggestion for improvement. [Package] wp-build /packages/wp-build labels Dec 23, 2025
Copy link
Member

@SirLouen SirLouen left a comment

Choose a reason for hiding this comment

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

Also forgot to mention to fix the linting error here

const handle = `${ externalConfig.handlePrefix }-${ shortName }`;

const packageJson = getPackageInfo( packageName );
const packageJson = getPackageInfo( packageName, args.resolveDir );
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const packageJson = getPackageInfo( packageName, args.resolveDir );
const packageJson = getPackageInfo(
packageName,
args.resolveDir
);

@youknowriad
Copy link
Contributor

Thanks for opening this PR @lezama I've opened an alternative PR here #74194 that should solve all the pnpm related issues.

@SirLouen
Copy link
Member

@youknowriad closing this as it got merged in #74194 right?

@SirLouen SirLouen closed this Dec 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package] wp-build /packages/wp-build [Type] Enhancement A suggestion for improvement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants