Skip to content
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

fix: always resolve runner's entry point #18013

Merged

Conversation

sheremet-va
Copy link
Member

@sheremet-va sheremet-va commented Sep 3, 2024

Description

Related: #16358 (reply in thread)

Passing down a non-resolved ID to the module runner works for loading files, but it creates invalid module cache entries that don't work with HMR.

cons runner = new ModuleRunner()

// all of the IDs are different from the module cache id
runner.import('./some-module')
runner.import('./some-module.ts')
runner.import('/some-module')

To resolve this, we always need to resolve entry points the same way Vite resolves URLs in the import analysis plugin. I see two possible solutions:

  1. Share the code between the import analysis plugin and fetchModule (implemented in this PR)
  2. Run the entry point module in some kind of "inlined" or "virtual" wrapper so the code is processed by Vite - this requires the user to define a plugin for this sort of thing, or we can have a built-in plugin that just injects an ID into an import, but this will probably break HMR in some way:
load(id) {
  return `export * from '${id}'`
}

Copy link

stackblitz bot commented Sep 3, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@bluwy
Copy link
Member

bluwy commented Sep 4, 2024

I don't particularly understand the usecase for fetchModule case yet, but I think this make sense. It would be nice if we can properly extract out normalizeUrl() from import analysis entirely in the future so it's something others could use too.

For the test, I think we can skip the hmr circular one for now, but I think it's correctly flagging a bug that we should look into later.

@sheremet-va
Copy link
Member Author

It would be nice if we can properly extract out normalizeUrl() from import analysis entirely in the future so it's something others could use too.

Agree. Vitest has actually a use case for this in the mocker.

@sheremet-va sheremet-va merged commit a836ea9 into vitejs:v6/environment-api Sep 4, 2024
6 of 8 checks passed
@sheremet-va sheremet-va deleted the fix/unresolved-entry-point branch September 4, 2024 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants