-
Notifications
You must be signed in to change notification settings - Fork 30
[chore] Simplify @expo/steps from dual ESM/CJS to plain CJS #658
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
Open
sjchmiela
wants to merge
10
commits into
main
Choose a base branch
from
sjchmiela/simplify-steps-to-cjs
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
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
- Remove 'type: module' field - Simplify main/types to use single dist/ directory - Remove dual exports field - Update build scripts to use simple tsc - Remove chokidar-cli (no longer needed for watch) - Remove this-file dependency (replaced with native __dirname)
- Update tsconfig.build.json to use commonjs module and output to dist/ - Delete tsconfig.build.commonjs.json (no longer need dual build) - Delete build.sh (replaced by simple tsc command in package.json)
Remove ESM-style .js extensions from all relative imports in source files. CommonJS module resolution doesn't require explicit extensions.
- Replace this-file createContext() with native __dirname - Remove import.meta.url usage in test files (use built-in __dirname) - Update SCRIPTS_PATH to point to dist/scripts instead of dist_commonjs/scripts
- Update cli.sh to reference dist/cli/cli.js instead of dist_commonjs/cli/cli.cjs - Simplify jest.config.cjs to use standard ts-jest preset (not ESM) - Remove moduleNameMapper for .js extensions (no longer needed) - Remove custom transform with useESM option
- Remove this-file import from runCustomFunction-test.ts - Update SCRIPTS_PATH reference to runCustomFunction.js (not .cjs)
hSATAC
approved these changes
Jan 12, 2026
Contributor
hSATAC
left a comment
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.
I am going to have a rebase hell after this was merged. 😭
Contributor
Author
|
oh no maybe |
Contributor
Author
|
hmm most of your changes are in |
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.
Why
The
@expo/stepspackage was configured with a dual ESM/CJS build setup that added unnecessary complexity:tsconfig.build.jsonandtsconfig.build.commonjs.json)build.shscript that usedsedto rename.jsfiles to.cjsthis-filepackage andimport.meta.url.jsextensions required on all importsThis complexity was not needed as the package runs in Node.js where CJS is well supported, and other packages in the workspace (like
@expo/eas-build-job) use plain CJS successfully and we're not planning to move off of CJS without moving all the rest of the packages to ESM too. Having this package as the only package with ESM enabled is a bit weird.How
"type": "module"frompackage.jsonmainandtypesfields pointing todist/tsconfig.build.jsonusingmodule: \"commonjs\"build.shscript and replaced with simpletsccommand.jsextensions from all import statements (~40 files)this-filepackage usage with native__dirnameimport.meta.urlwith__dirnameequivalentsts-jestpresetcli.shto point todist/cli/cli.jsthis-fileandchokidar-clidev dependenciesTest Plan
yarn buildcompletes successfullyyarn testpasses all 281 tests across 20 test suites