-
Notifications
You must be signed in to change notification settings - Fork 386
[CLI] Fix __dirname not defined error in intl extension #3094
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
Conversation
Use conditional logic to determine the module directory: prefer __dirname when available (CommonJS), fall back to import.meta.dirname (ESM). Also switch to path.join for more robust path construction.
| const dataPath = `${__dirname}/shared/${dataName}`; | ||
| const moduleDir = | ||
| typeof __dirname !== 'undefined' ? __dirname : import.meta.dirname; | ||
| const dataPath = path.join(moduleDir, 'shared', dataName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to fix the missing __dirname bug, this PR switches from string concat to explicit path concat.
Confirmed. This issue was introduced by 74446e7. The tests on that commit encountered the error but somehow did not fail the CI job: Looking into it. |
|
Our ES module tests for built NPM packages did not consider timeouts to be failures. That should be fixed now. |
|
Nice! The built package tests are failing due to the missing Now that the __dirname fix is restored, we can merge this once the latest CI tests pass. I have to go AFK shortly and will wait until I am back in 3-4 hours to merge and publish new NPM packages. That way, there is opportunity to confirm all is well with the latest version before moving on to other things. To others on the team, please feel free to merge and publish before then. The latest published version of Playground CLI is currently broken. |
|
@brandonpayton Thank you! |
Motivation for the change, related issues
Running
npx @wp-playground/cli server --debugcurrently fails for v3.0.38.I think our tests for built npm packages should have caught this before merging, so I would like to look into that before merging this fix.
cc @mho22
Implementation details
Use conditional logic to determine the module directory: prefer __dirname when available (CommonJS), fall back to import.meta.dirname (ESM). Also switch to path.join for more robust path construction.
Testing Instructions (or ideally a Blueprint)