-
Notifications
You must be signed in to change notification settings - Fork 140
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
re-enable watch tests #2126
base: main
Are you sure you want to change the base?
re-enable watch tests #2126
Conversation
910fe3b
to
6c835af
Compare
6c835af
to
f14f3b8
Compare
f14f3b8
to
4f70b3b
Compare
Maybe using something like virtual pair component for template only would help to reduce code in the hbs plugin and then it would probably be easier to support hmr. |
@mansona I fixed the particular tests that we were looking at. There are other tests that need cleanup now. For example "Scenario 4" is assuming that it will see a fresh app, but the command watcher is shared between tests, so it's initial condition fails because hello-world.js already exists from another test. |
} | ||
|
||
// we should be able to clear any earlier meta by returning | ||
// resolution.meta here, but vite breaks that rollup feature. |
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.
Just use own cache then?
MetaCache = new Map();
And set the value for the id in the resolveId
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.
We could but this is working now. I checked and the rollup docs say it's OK to mutate the meta object that comes back from getModuleInfo().meta.
@ef4 I looked at the tests and I think its missing the case |
# Conflicts: # packages/vite/src/hbs.ts
This PR now updates the watch tests so they work as expected in the setup, but there is still a problem. I think we've identified a regression that will need to be fixed 🤔
Fixes: embroider-build/app-blueprint#22