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: add workaround to fix build-temp-types on Netlify #7495

Merged
merged 2 commits into from Mar 28, 2022

Conversation

fwouts
Copy link
Contributor

@fwouts fwouts commented Mar 28, 2022

Description

This fixes an issue that occurs in the Netlify build, as reported by @patak-dev in #6856 (comment)

This is caused by postcss-load-config missing postcss as a peer dependency, causing a crash when running tsc since it cannot find the corresponding type definitions because of pnpm's strictness. Thank you @bluwy for the idea!


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 patak-dev requested a review from bluwy March 28, 2022 13:37
@fwouts
Copy link
Contributor Author

fwouts commented Mar 28, 2022

I can also send a PR for postcss-load-config but I won't have time for a couple of days at least (FYI @ai).

Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

Neat workaround!

@ai
Copy link

ai commented Mar 28, 2022

postcss-load-config is using postcss only in types https://github.com/postcss/postcss-load-config/blob/main/src/index.d.ts#L4=

Not in the code.

@patak-dev patak-dev merged commit 01bc415 into vitejs:main Mar 28, 2022
@patak-dev
Copy link
Member

@fwouts thanks for the PR, I can confirm that this fixed the issue in Netlify

@bluwy
Copy link
Member

bluwy commented Mar 28, 2022

@ai Vite bundles the types too so it needs to be a peer dep to be bundled properly. Maybe that's out of scope for the package, but I think it's fair to still add an optional peer dep for it so there's an explicit link instead on relying on hoisted types. It could help postcss-load-config too if there are changes to postcss's types.

@ai
Copy link

ai commented Mar 28, 2022

Optional peer is a good option. Send PR when you will have time.

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.

None yet

4 participants