Skip to content

Upgrade babel deps #530

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.

Sign up for GitHub

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
NullVoxPopuli wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Upgrade babel deps #530

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d9764b
Upgrade @babel/*, babel-
NullVoxPopuli Feb 4, 2026
480ba7e
Be more clear about what we're composing, and how it is correct
NullVoxPopuli Feb 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/relative-module-paths.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ function getRelativeModulePath(modulePath) {
}

function resolveRelativeModulePath(name, child) {
name = name.replace(/\.js$/, '');

@NullVoxPopuli NullVoxPopuli Feb 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is needed because @babel/runtime is a "type": "commonjs" package that uses extensions -- which node supports but broccoli does not.

@void-mAlex void-mAlex Feb 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is that still true for babel8?

@NullVoxPopuli NullVoxPopuli Feb 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

babel 8 is ESM only, so I don't know if it would be compatible with broccoli -- maybe more hacks needed

@mansona mansona Feb 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the latest broccoli only supports node 20.19 because it relies on require(esm). We're at the point where we can start doing the same in the wider ecosystem. This might be worth discussing at the next tooling team meeting

Also there is nothing stopping cjs from supporting extensions 🤔 what is the specific error here that the change is trying to fix?

@NullVoxPopuli NullVoxPopuli Feb 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

here is what the failure looks like: #531

@NullVoxPopuli NullVoxPopuli Feb 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

image

js is already a default extension

https://github.com/tleunen/babel-plugin-module-resolver/blob/master/src/normalizeOptions.js#L11

I'll debug it, but this makes me despair lol

@NullVoxPopuli NullVoxPopuli Feb 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no, thee fix does not go in babel-plugin-module-resolver.

we override resolving logic with the function I changed -- so this is our own doing, and our own fix.

There is this: https://npmx.dev/package/amd-name-resolver which I also think is the wrong place to the fix.

The only place I think a deeper fix makes sense is loader.js.

or we lean in to composition, and because we have this need to eliminate extensions (and we don't want to encourage the broader ecosystem to use AMD), we just fix it here in ember-cli-babel.

The whole of moduleResolve could probably go away tho... because we have require resolve, and the URL utils.

but that feels like a bigger effort for another PR, rather than a dep-upgrade PR

@NullVoxPopuli NullVoxPopuli Feb 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

tl;dr: I think fixing in ember-cli-babel is correct

@NullVoxPopuli NullVoxPopuli Feb 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

especially since the alternative is to reach in to private code and call babel-plugin-module-resolver's own stripExtensions:

function stripExtension(modulePath, stripExtensions) {
  let name = _path.default.basename(modulePath);
  stripExtensions.some(extension => {
    if (name.endsWith(extension)) {
      name = name.slice(0, name.length - extension.length);
      return true;
    }
    return false;
  });
  return name;
}

which is about as safe as what I have in this PR -- albeit likely slower

@NullVoxPopuli NullVoxPopuli Feb 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I did PR this to the upgrade branch tho: #532

return moduleResolve(name, getRelativeModulePath(child));
}

Expand Down
26 changes: 13 additions & 13 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,19 @@
"test:node:debug": "mocha debug node-tests"
},
"dependencies": {
"@babel/helper-compilation-targets": "^7.20.7",
"@babel/plugin-proposal-decorators": "^7.20.13",
"@babel/plugin-transform-class-properties": "^7.16.5",
"@babel/plugin-transform-class-static-block": "^7.22.11",
"@babel/plugin-transform-modules-amd": "^7.20.11",
"@babel/plugin-transform-private-methods": "^7.16.5",
"@babel/plugin-transform-private-property-in-object": "^7.20.5",
"@babel/plugin-transform-runtime": "^7.13.9",
"@babel/plugin-transform-typescript": "^7.20.13",
"@babel/preset-env": "^7.20.2",
"@babel/runtime": "7.12.18",
"@babel/helper-compilation-targets": "^7.28.6",
"@babel/plugin-proposal-decorators": "^7.29.0",
"@babel/plugin-transform-class-properties": "^7.28.6",
"@babel/plugin-transform-class-static-block": "^7.28.6",
"@babel/plugin-transform-modules-amd": "^7.27.1",
"@babel/plugin-transform-private-methods": "^7.28.6",
"@babel/plugin-transform-private-property-in-object": "^7.28.6",
"@babel/plugin-transform-runtime": "^7.29.0",
"@babel/plugin-transform-typescript": "^7.28.6",
"@babel/preset-env": "^7.29.0",
"@babel/runtime": "^7.28.6",
"amd-name-resolver": "^1.3.1",
"babel-plugin-debug-macros": "^0.3.4",
"babel-plugin-debug-macros": "^2.0.0",
"babel-plugin-ember-data-packages-polyfill": "^0.1.2",
"babel-plugin-ember-modules-api-polyfill": "^3.5.0",
"babel-plugin-module-resolver": "^5.0.0",
Expand All @@ -69,7 +69,7 @@
"semver": "^7.3.8"
},
"devDependencies": {
"@babel/core": "^7.22.10",
"@babel/core": "^7.29.0",
"@ember/optional-features": "^2.0.0",
"@ember/string": "^3.1.1",
"@ember/test-helpers": "^3.2.0",
Expand Down
Loading