fix(node): remove timestamp query of staticImportedUrls

[chaejunlee]

Description

fixes #15607

I would like to add some tests for it, but I am not sure how to make the imports to have the timestamp query.

The playground/hmr/soft-invalidate's imports don't seem to have timestamp query when imported.

I'll try to find it my own, but if anyone can show me the way, I'll add some tests as well.

Does this issue happen because of how .vue parses the imports?

Because in playground/hmr/soft-invalidate, changing child.js after changing index.js triggers soft-invalidation of index.js and it is correctly updates the value of foo.

Additional context

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines, especially the Pull Request Guidelines.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Update the corresponding documentation if needed.
• Ideally, include relevant tests that fail without this PR but pass with it.

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[chaejunlee]

I think this is more of a problem of changes not propagating to the entry file, such as main.ts.

In the example of playground/hmr, when we update any file, the entry file hmr.ts gets updated and produce new timestamped file.

However, for the repro of the issue, the changes don't propagate up to the entry file main.ts.

What do you think?

[chaejunlee]

I can confirm that the fix that I made doesn't really solve the issue.

I could see a major difference of App.vue when updating it.

I think this might be the source of this issue, but I think this is more of a vite-plugin-vue or vue itself.

[YellRes]

I can confirm that the fix that I made doesn't really solve the issue.

I could see a major difference of App.vue when updating it.

I think this might be the source of this issue, but I think this is more of a `vite-plugin-vue` or `vue` itself.

after change app.vue, app.vue will reload and transform iteself. so a major difference app.vue will come out, this is right.
but if change app.vue, then change a.ts, a major difference app.vue will never come out. that what the fix usage.

[chaejunlee]

Ah now that I see, you are right. However, to me, my fix doesn't look like it works. Can you pickup my fix and apply to yours? I don't really use vue so I can't tell.

[chaejunlee]

When vue calls rerender, it reuses initial component's state.

https://github.com/vuejs/core/blob/ee4cd78a06e6aa92b12564e527d131d1064c2cd0/packages/runtime-core/src/hmr.ts#L78-L85

Where calling reload, it updates the component, also its state.

https://github.com/vuejs/core/blob/ee4cd78a06e6aa92b12564e527d131d1064c2cd0/packages/runtime-core/src/hmr.ts#L102-L108

In vite-plugin-vue, once the App.vue is edited, _rerender_only is defined and hmr only calls rerender.

I don't think this is a vite problem. I have followed @bluwy 's instruction. However, because App.vue depend on timestamped a.ts, it doesn't help.

[chaejunlee]

My thought of fix would be adding a more condition to defining rerender_only.

https://github.com/vitejs/vite-plugin-vue/blob/1963dcf64e89a4955fadc9817168de17aa34501f/packages/plugin-vue/src/main.ts#L152-L154

to something like

function hasVariable(template) {
  return template.includes("{{");
}
if (prevDescriptor && isOnlyTemplateChanged(prevDescriptor, descriptor) && !hasVariable(descriptor.template)) {
  output.push(`export const _rerender_only = true`);
}

Or something like

// some how inject the filename (changedFilename) that caused hmr
const isDependencyChanged = changedFilename !== filename; 
if (prevDescriptor && isOnlyTemplateChanged(prevDescriptor, descriptor) && !isDependencyChanged) {
  output.push(`export const _rerender_only = true`);
}

Though, I don't really know because it is not related to vite anymore.

[YellRes]

My thought of fix would be adding a more condition to defining rerender_only.

https://github.com/vitejs/vite-plugin-vue/blob/1963dcf64e89a4955fadc9817168de17aa34501f/packages/plugin-vue/src/main.ts#L152-L154

to something like

function hasVariable(template) {
  return template.includes("{{");
}
if (prevDescriptor && isOnlyTemplateChanged(prevDescriptor, descriptor) && !hasVariable(descriptor.template)) {
  output.push(`export const _rerender_only = true`);
}

Or something like

// some how inject the filename (changedFilename) that caused hmr
const isDependencyChanged = changedFilename !== filename; 
if (prevDescriptor && isOnlyTemplateChanged(prevDescriptor, descriptor) && !isDependencyChanged) {
  output.push(`export const _rerender_only = true`);
}

Though, I don't really know because it is not related to vite anymore.

you are right, i think there is two bugs here:

• in vite, app.vue should soft invalidate itself
• in @vitejs/plugin-vue, once change the a.ts, after the hmr, app.vue should the reload itself

so i think, this fix only focus vite, which make sure app.vue can soft invalidate itself.
as for the other bug, i think it should be fixed in @vitejs/plugin-vue.

[bluwy]

@chaejunlee Sorry for the late reply (and leading you to a rabbithole). I just tested the repro, and the soft-invalidate playground, and it seems like the fix is working as expected. I don't quite understand why Vue is causing an issue here?

For example, in the soft-invalidate playground before this PR, you can try to edit child.js first, then index.js, then child.js again. I can see that index.js is incorrectly transformed 3 times due to the message on the page (should be twice, one on init, one on index.js update only):

soft-invalidation/indedd.js is transformed 3 times. child is bar

Perhaps I'm missing something about the Vue case here? The imports are currently only parsed by us, so I think Vue shouldn't be interfering it.

I would like to add some tests for it, but I am not sure how to make the imports to have the timestamp query.

When you update child.js for the first time, index.js should contain the imports to it with the timestamps.

[chaejunlee]

@bluwy No worries. I respect your time.

I think I was too focused on the issue and didn't see what the fix actually does. I think what you found out of the issue doesn't necessarily solve the issue but it was definitely a bug. Thanks for pointing it out. I have added a test to verify the fix.

I think this fix is what vite can do for the issue. What I found out is that vue-plugin tries to cache the import variables hard and it causes the OP's issue.

[bluwy]

Ahhh I see what you mean. I didn't notice that Vue isn't picking up the new values and re-render it. I was only verifying via the transform() hook call count.

That definitely looks like a @vitejs/plugin-vue bug then. You can open a new issue there if you'd like. This PR looks good for now for Vite 👍

[patak-dev]

I think we may be also missing a removeImportQuery() here, we can add that in another PR though. You can search the codebase for this function and you'll see we are removing ?import before transforming and when unwrapping ids to check the module graph. I think these we need better handling in general of these two functions though so we can merge for now this PR.