-
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
vite: fix hbs loading for virtual pair components #1813
Conversation
This is likely going to be solved as part of #1794, we literally just hit a similar error |
@mansona i think that will fix a specific issue. But not the filter. I think the filter is not good. |
1e33bc3
to
59b42eb
Compare
634b4f0
to
9c880bb
Compare
first error https://github.com/embroider-build/embroider/actions/runs/8252373750/job/22571786702?pr=1813#step:4:259 second: |
7bc2c27
to
c11bfac
Compare
c11bfac
to
d7e9625
Compare
templateResolution.id = resolvedId; | ||
|
||
const pkg = resolverLoader.resolver.packageCache.ownerOfFile(resolvedId); | ||
const isInComponents = pkg?.isV2App() && resolvedId.slice(pkg?.root.length).startsWith('/components'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will always be false for addons, which is good, because they should not do template colocation / template-only.
It's already done in previous stage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good 👍 myself and @ef4 reviewed it this morning and we're happy to get it merged. I added a commit to reset the hbs extension handling to use createFilter again, but tests pass and I'm happy that it's doing the same thing
it happens when embroider generates a virtual pair component, the resolved id then looks like
/template.hbs/component.ts-embroider-pair-component