-
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
components in v2 addon's app-js using index files broken #1835
Comments
I think this can be customized via: addon.appReexports(['**/*.js'], { mapFilename: (filename) => {
if (filename.includes('/components/') && filename.endsWith('index.js')) {
return path.dirname(filename) + '.js';
}
} }), does that work? See also prior discussions: tl;dr: we opted not to resolve this at the resolver layer, since it's an explicit |
So this was recently fixed on main and it's likely just a bug in the component resolver. I could see some value fixing this on stable but I think we have diverged a bit much to be able to simply backport the change 🤔 |
@mansona are you able to point me to where this was fixed on |
Yeah, I know this workaround exists. But the question in this issue was more where to properly fix this? Is this a bug in Embroider (seems so, as Chris confirmed), or should we bake that behaviour into the rollup plugin? I'd rather not want to introduce this workaround everywhere in user-land code, since index-modules is a supported thing. |
Not in ESM node 😅 (which is a bit of an oversight for co-location -- I also learned this by running in to broken stuff (and as it turns out, this is what |
Given index-modules based file layout for colocated component files as supported by RFC481 in a v2 addon:
And generating the package's
app-js
by the rollup plugin provided by@embroider/addon-dev
:when building an app consuming the addon using Embroider v3.4.5 and "optimized" settings, it would fail with:
It does work work with either
It will also work in Embroider v3.4.5 "optimized" when changing the app-js's left side to not use index modules:
So we could fix this by exposing the app-js not using index modules, but since index-modules are officially supported by RFC481, it looks like a bug in Embroider?
The text was updated successfully, but these errors were encountered: