Skip to content

fix: build.modulePreload.resolveDependencies is optimizable#16083

Merged
patak-cat merged 4 commits into
vitejs:mainfrom
sapphi-red:feat/using-resolveDependencies-is-optimizable
Aug 9, 2024
Merged

fix: build.modulePreload.resolveDependencies is optimizable#16083
patak-cat merged 4 commits into
vitejs:mainfrom
sapphi-red:feat/using-resolveDependencies-is-optimizable

Conversation

@sapphi-red

Copy link
Copy Markdown
Member

Description

The document of build.modulePreload.resolveDependencies had

Returning a relative path to the hostId for hostType === 'js' is allowed, in which case new URL(dep, import.meta.url) is used to get an absolute path when injecting this module preload in the HTML head.

, but actually this wasn't true. build.modulePreload.resolveDependencies does not expect a relative path to the hostId. It expects a relative path to build.outDir. The result of build.modulePreload.resolveDependencies is passed to toOutputFilePathWithoutRuntime or toOutputFilePathInJS.

close #13169 (indirectly)

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.

@sapphi-red sapphi-red added the p3-minor-bug An edge case that only affects very specific usage (priority) label Mar 3, 2024
@sapphi-red sapphi-red requested a review from patak-cat March 3, 2024 14:38
@bolt-new-by-stackblitz

Copy link
Copy Markdown

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


renderedDeps = resolvedDeps.map((dep) => {
let renderedDeps: number[]
if (renderBuiltUrl) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If modulePreload.resolveDependencies returns a relative path to build.outDir, then it should be possible to use the else block.

Comment thread docs/config/build-options.md Outdated
Comment thread packages/vite/src/node/plugins/importAnalysisBuild.ts Outdated
bluwy
bluwy previously approved these changes May 13, 2024
@patak-cat

Copy link
Copy Markdown
Member

/ecosystem-ci run

@vite-ecosystem-ci

Copy link
Copy Markdown

@sapphi-red

sapphi-red commented May 31, 2024

Copy link
Copy Markdown
Member Author

Resolved conflicts 👍
I guess the vite-plugin-vue fail happened because the branch was old.

@sapphi-red

Copy link
Copy Markdown
Member Author

/ecosystem-ci run

@vite-ecosystem-ci

Copy link
Copy Markdown

@patak-cat patak-cat merged commit e961b31 into vitejs:main Aug 9, 2024
@sapphi-red sapphi-red deleted the feat/using-resolveDependencies-is-optimizable branch August 10, 2024 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p3-minor-bug An edge case that only affects very specific usage (priority)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

importAnalysisBuild.ts injects same CSS styles multiple times

3 participants