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(build): resolve rollupOptions.input paths #5601

Merged
merged 11 commits into from
Nov 11, 2021

Conversation

sibbng
Copy link
Contributor

@sibbng sibbng commented Nov 9, 2021

Description

Related unocss/unocss#88

Entry points defined in rollupOptions.input passed to rollup without resolving. This causing some path issues in Windows.

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.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • 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).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@patak-dev
Copy link
Member

Would it be possible to add a test case to avoid future regressions?

@sibbng
Copy link
Contributor Author

sibbng commented Nov 10, 2021

Would it be possible to add a test case to avoid future regressions?

Done 👍

antfu
antfu previously approved these changes Nov 10, 2021
@patak-dev patak-dev merged commit 5b6b016 into vitejs:main Nov 11, 2021
@patak-dev
Copy link
Member

patak-dev commented Nov 11, 2021

Thanks for doing the refactors in this PR @sibbng 👍🏼

@sibbng
Copy link
Contributor Author

sibbng commented Nov 11, 2021

You're welcome @patak-js 😉

@sibbng sibbng deleted the resolve-input branch November 11, 2021 13:49
sibbng added a commit to sibbng/vite that referenced this pull request Nov 14, 2021
@sibbng sibbng mentioned this pull request Nov 14, 2021
9 tasks
patak-dev pushed a commit that referenced this pull request Nov 14, 2021
@IanVS
Copy link
Contributor

IanVS commented Dec 6, 2021

Just a note, a project that I help out with, https://github.com/eirslett/storybook-builder-vite, was using an unresolved rollupOptions.input and relying on the fact that it was not resolved later on in the plugin. Maybe this is enough of a change to call out in the 2.7.0 migration docs for plugin authors to take into account?

@khalwat
Copy link
Contributor

khalwat commented Dec 21, 2021

@sibbng Regarding this change: 3127219#diff-e861d33a288ea61b3aee05203d1c43920c4326d904e3d41b4326adbd30a0127f

I can see the intent to preface the virtual module with an id of 0 but the result after going through a build process is it is prefaced with the Unicode character for null (\u0000).

I wanted to see if this was intentional, or perhaps some unexpected side-effect?

The relevant discussion is here: ElMassimo/vite_ruby#156

We're trying to assess how backend systems should be looking for and loading the vite/legacy-polyfills

@patak-dev
Copy link
Member

@sibbng we are having issues with the added resolving. See #6576. IIUC, what you needed was path normalization instead of resolving, no? Could you take a look at #6576? It is valid to pass this to rollup, input can even be a virtual path.

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.

5 participants