-
Notifications
You must be signed in to change notification settings - Fork 142
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
v1 addon with gts + ember-template-imports failing under Embroider #2119
Comments
is the babel plugin babel-plugin-ember-template-compilation running there? I think there was a condition to only include it if there are any hbs files. I might be wrong |
found it: embroider/packages/compat/src/v1-addon.ts Line 199 in fae4059
It says it's only enabled for inline hbs, i think gjs/gts is kind of it |
this is what I did to get it to work: plugins: [
require.resolve('ember-auto-import/babel-plugin'),
[
require.resolve('./babel-plugin-ember-template-compilation.js'), {
compilerPath: require.resolve('ember-source/dist/ember-template-compiler.js'),
targetFormat: 'hbs',
enableLegacyModules: [
'ember-cli-htmlbars',
'ember-cli-htmlbars-inline-precompile',
'htmlbars-inline-precompile',
'@ember/template-compilation'
],
transforms: [],
}] |
hi @patricklx Is that added to the babel section of |
yes, you need to create the file module.exports = require('babel-plugin-ember-template-compilation') |
yes that compiles. thank you. it does not resolve my initial issue though... |
so, this is to fix the issue in v1 addons, so the change needs to be done in the addon main entry file. |
|
the problem is that babel-plugin-ember-template-compilation wasn't included. it probably will be with the next release of ember-template-imports |
That's not going to have any effect. Embroider doesn't let v1 addons decide whether or not to do their own template compilation. But yeah, I'm starting to understand. I'll followup on the PR. |
I wrote up more explanation in #2120 (comment) and this is a bug we can fix. But also: it has long been recommended that v1 addon authors publish JS and not TS. Even ember-cli-typescript gave a It's supported and we can keep fixing bugs, but please spread the word. People should not be publishing GTS (or GJS) to NPM. Compile it to javascript once instead of making every app author do it hundreds of times a day. As for how to do that in a v1 addon: if I needed to do that I would probably start with the v2 blueprint and modify it slightly to publish in v1. The smallest thing that would probably work is:
Now you have a valid v1 addon that's all pre-built with rollup and doesn't need a dependency on ember-template-imports or typescript. |
in my case it are in repo (and private) addons. so i guess the js over ts argument does not apply here... |
the js over ts argument does still apply because we're talking about module boundaries regardless if you're actually "publishing to npm". I have encouraged many people to imagine they are publishing to npm anyway and it has very often solved many problems for them 🎉 |
sure, yet I really want to write TS... :-) |
He’s not saying to stop using TS. He’s saying to build each library’s TS to JS separately, so that all other packages only need to see the JS (and .d.ts). |
I don't want to argue over whether publishing JS vs. TS is better, since I do think you are probably right. But also I wouldn't want to spend time on making a v1 addon pre-compile TS, given that most of our addons are v2 already, and the effort migrating to v2 is probably better spent where this is not the case. However, I don't think you are right about what has been recommended for v1 addons, or can you point me to a place where this has been recommended? ember-cli-typescript's addon docs say the opposite:
And
|
Should be fixed by #2120 |
Oh, I was mistaken then, I thought it also published the .js. |
When a v1 addon has gts components and ember-template-imports, this is working under Classic, but failing under Embroider (stable). Imports of components used in
<template>
get stripped away (in the rewritten package), thereby leading toAttempted to invoke a component that was not in scope in a strict mode template
errors.Found some prior discussion around that on Discord between @Windvis and @NullVoxPopuli, where it was suggested to make the v1 babel typescript config same as for v2 addons. Which @Windvis did here, and I can confirm in my own case that adding
onlyRemoveTypeImports
fixes the issue.However, we should get this to work out of the box, right? It was suggested to add this config to ember-cli-babel by default, but not sure if that is viable? Couldn't this break stuff when imports are only used for types, but not declared explicitly with e.g.
import type
?Also puzzling to me is the fact that this only causes errors under Embroider? (which is the reason I opened this issue here, even when the fix might go somewhere else)
The text was updated successfully, but these errors were encountered: