Skip to content

fix: add workaround to fix build-temp-types on Netlify#7495

Merged
patak-cat merged 2 commits intovitejs:mainfrom
fwouts:netlify-fix
Mar 28, 2022
Merged

fix: add workaround to fix build-temp-types on Netlify#7495
patak-cat merged 2 commits intovitejs:mainfrom
fwouts:netlify-fix

Conversation

@fwouts
Copy link
Copy Markdown
Contributor

@fwouts fwouts commented Mar 28, 2022 •
edited
Loading

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-cat patak-cat requested a review from bluwy March 28, 2022 13:37
@fwouts
Copy link
Copy Markdown
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).

bluwy
bluwy approved these changes Mar 28, 2022
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown

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-cat patak-cat merged commit 01bc415 into vitejs:main Mar 28, 2022
@patak-cat
Copy link
Copy Markdown
Member

patak-cat commented Mar 28, 2022

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

@bluwy
Copy link
Copy Markdown
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
Copy Markdown

ai commented Mar 28, 2022

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

@bluwy bluwy mentioned this pull request Mar 29, 2022
Merged
2 tasks
@brc-dd brc-dd mentioned this pull request Nov 30, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bluwy bluwy bluwy approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@fwouts @ai @patak-cat @bluwy