-
-
Notifications
You must be signed in to change notification settings - Fork 42
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
set moduleName to relativePath #173
Conversation
+1 to this, having the full filepath as moduleName can cause issues between builds via broccoli-caching-writer caching build output if the tree is unchanged. This is problematic if you have a project with multiple addons using workspaces, and build addons independently. Building two dependent addons back to back will cause the second build to use the cached build output from the first, with a now incorrect moduleName. |
@ef4 I think this is a better solution to the problem behind emberjs/babel-plugin-ember-template-compilation#13 |
I'll work on this tomorrow |
I'm still in holidays, so cannot run tests, but i think it was only a linting issue. The tests do not check state in between. Only final html output |
ah just noticed the __tests__ folder... 😅 |
set moduleName to relativePath, like its done in [ember-cli-htmlbars](https://github.com/ember-cli/ember-cli-htmlbars/blob/master/lib/template-compiler-plugin.js#L22) otherwise it will use the filename, which might be less helpful, especially when using pnpm the paths will be too long.
@dfreeman failed some scenarios, but errors where unrelated to this pr |
@ef4 if content-tag is used, this fix would get lost. I would suggest that content-tag takes also a parameter like moduleName that can be passed as options to template call and then be parsed by babel compiler |
I'm not sure if that's true, the output of content-tag still goes through babel-plugin-ember-template-compilation. I think that will handle moduleName. But also: it is fundamentally misleading to use moduleName with template tag anyway, because one module can contain many template tags. |
Yes, but the ember template compiler sets it to the filename if nothing is passed.... |
I do not see where babel-plugin-ember-template-compilation handles it. It would probably also needs to be updated to detect moduleName in the template call params. Or does it pass all other properties to precompileTemplate? |
Maybe i misunderstood, what i meant is that content-tag needs an option to let it know the moduleName, so it can set it to the template params property, then babel-plugin-ember-template-compilation can handle it and pass it to precompileTemplate |
@ef4 also found this: https://rfcs.emberjs.com/id/0931-template-compiler-api/#C11_L16 |
@patricklx can you rebase? I think I deleted one of the files you changed 😅 |
this would be needed to be done in content-tag now. Closing |
set moduleName to relativePath, like its done in ember-cli-htmlbars otherwise it will use the filename, which might be less helpful (and I argue that the filename is not a moduleName, although it can be, but with all the (possible) linking (symbolic, hard link etc) in between I do not think it makes sense), especially when using pnpm the paths will be too long. see emberjs/ember-inspector#2425