Skip to content

Does @babel/runtime still need pin? #519

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
gorner opened this issue Mar 12, 2025 · 6 comments
Open

Does @babel/runtime still need pin? #519

gorner opened this issue Mar 12, 2025 · 6 comments

Comments

@gorner
Copy link

gorner commented Mar 12, 2025

@babel/runtime was pinned to v7.12.8 in #385 (March 2021) due to 'workaround test failures in CI for the "Floating dependencies" scenario.'

Is this pin still needed / relevant? As it stands, this appears to block fully updating @babel/runtime to v7.26.10 in an Ember app using ember-cli-babel (see GHSA-968p-4wvh-cqc8). Alternatively we could use NPM overrides, but this is less ideal and I'll need to check there isn't some compatibility issue.

@gorner
Copy link
Author

gorner commented Mar 13, 2025

I briefly looked at opening a PR myself to at least change the @babel/runtime dependency to "^7.26.10" but ran into issues:

  • If I only change the one dependency, mocha node-tests passes but ember test fails with:
    Uncaught Error: Could not find module `@babel/runtime/helpers/esm/toPropertyKey.js` imported from `@babel/runtime/helpers/esm/defineProperty`
    
  • If I remove yarn.lock and reinstall dependencies from scratch, one mocha test (can compile static blocks) fails, and ember test still fails for the same reason as before.

Using npm overrides in an Ember app with "@babel/runtime": "^7.26.10", does not seem to cause any issues in the app I work on, at least after some quick smoke testing.

However I see now that there is an existing issue #442 suggesting that there are/were some known compatibility issues (which appears similar to the one I saw in ember test) preventing it from being unpinned before.

@gorner
Copy link
Author

gorner commented Mar 17, 2025

Looks like the underlying issue with @babel/runtime v7.13+ is described here:

Basically babel is including the extension in the import paths for the helpers, and our transpilation doesn't support modules at runtime including the extension.

Unfortunately I'm not in a position to investigate this further right now, just trying to point the way in case there is someone with an idea of how to fix this.

@gorner
Copy link
Author

gorner commented Apr 10, 2025

So I've figured out a potential fix, at least in POC form. But as far as I can tell, it requires a change to https://github.com/ember-cli/loader.js to allow imports with a .js extension to resolve to the correct module that was loaded without that extension. After linking that into the ember-cli-babel PR branch I mentioned above, at least the Ember tests that were failing previously now pass.

I have a draft PR with that change here: ember-cli/loader.js#227 — but it only handles the .js case and might need to be more robust. Any guidance from the Ember/CLI core teams on how to proceed or if it's worth pursuing this would be welcome.

edit: however @babel/runtime updates might also be blocked by #442

2nd edit: I eventually tried the reproduction app linked from #442 and could not reproduce if I applied my loader.js change. So maybe that would fix it?

@ef4
Copy link
Collaborator

ef4 commented May 20, 2025

I commented here #442 (comment)

But the path forward for people hitting this is to stop using the includeExternalHelpers feature. It's not enabled by default. If you (or one of your addons) don't enable it, you don't get this problem. And it's a pretty bad feature that forces all of @babel/runtime to be eagerly included in your build, even the many parts you don't need.

The linked comment explains how to use @babel/runtime directly, without all of the broken shenanigans in ember-cli-babel.

We do not want to make a major behavior change to loader.js or ember-cli-babel because both are effectively stable legacy code. The v2 app blueprint uses neither.

@gorner
Copy link
Author

gorner commented May 20, 2025

OK, thanks @ef4. So if I'm understanding correctly, the solution to clearing the advisory is:

  1. ensure your app/addons don't use includeExternalHelpers (I've confirmed mine do not)
  2. follow the steps in your comment on #442 to depend directly on @babel/runtime
  3. at that point it would be safe to use NPM overrides or equivalent if necessary to clear the old runtime from the build tree.

Please feel free to correct if I've misunderstood anything.

@gorner
Copy link
Author

gorner commented May 20, 2025

Depending explicitly on latest @babel/runtime and adding it to overrides seems to work and clears the security advisory for my app.

FWIW, ef4's above-linked comment also suggests adding '@babel/plugin-transform-runtime' to babel plugins, but this caused an error "Could not find module @babel/runtime/helpers/initializerDefineProperty" for me. It seems to be irrelevant to clearing the advisory anyways unless you were using includeExternalHelpers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants