Skip to content
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

Ember: Fix ember-template-compiler import for ember 6+ #30682

Open
wants to merge 3 commits into
base: next
Choose a base branch
from

Conversation

leoeuclids
Copy link

@leoeuclids leoeuclids commented Feb 26, 2025

What I did

Added the .js extension to the ember-template-compiler import, in the framework-preset-babel-ember.js file.
This change makes it compatible with ember-source@6 and above while keeping compatibility for older versions.

Reasoning

After the package.json changed in ember-source@6, it now requires the extension to properly find the file to be imported.

Greptile Summary

This PR adds the .js extension to the ember-template-compiler import in the Ember framework preset, ensuring compatibility with ember-source@6+ while maintaining support for older versions.

  • Modified code/frameworks/ember/src/server/framework-preset-babel-ember.ts to include the .js extension in the import statement for ember-template-compiler
  • The change addresses a breaking change in ember-source@6's package.json that now requires file extensions for imports
  • Simple but critical fix that maintains backward compatibility while supporting newer Ember versions

💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

LGTM

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

LGTM

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

@gossi
Copy link
Contributor

gossi commented Mar 1, 2025

will it also work for ember < 6 ?

Copy link

nx-cloud bot commented Mar 2, 2025

View your CI Pipeline Execution ↗ for commit 0734036.

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 1m 54s View ↗

☁️ Nx Cloud last updated this comment at 2025-03-09 01:10:01 UTC

@leoeuclids
Copy link
Author

will it also work for ember < 6 ?

Yep, it's backward compatible, at least with version 5 (I've manually tested with this version), but should technically work for older versions as well

@shilman shilman changed the title Fix ember-template-compiler import for ember 6+ Ember: Fix ember-template-compiler import for ember 6+ Mar 3, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

LGTM

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

@storybook-pr-benchmarking
Copy link

storybook-pr-benchmarking bot commented Mar 3, 2025

Package Benchmarks

Commit: 0734036, ran on 9 March 2025 at 01:28:37 UTC

No significant changes detected, all good. 👏

@leoeuclids
Copy link
Author

Hey @valentinpalkovic @shilman, anything missing for merging this? Please let me know if I can help in any way.

@shilman
Copy link
Member

shilman commented Mar 9, 2025

Hi @leoeuclids !! Thanks so much for putting this together. It's currently generating the following CI error. Any ideas?

src/server/framework-preset-babel-ember.ts:4:28 - error TS7016: Could not find a declaration file for module 'ember-source/dist/ember-template-compiler.js'. '/tmp/storybook/code/node_modules/ember-source/dist/ember-template-compiler.js' implicitly has an 'any' type.
Try npm i --save-dev @types/ember-source if it exists or add a new declaration (.d.ts) file containing declare module 'ember-source/dist/ember-template-compiler.js';

4 import { precompile } from 'ember-source/dist/ember-template-compiler.js';

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

Successfully merging this pull request may close these issues.

4 participants