Download platform specific package if optionalDependencies are skipped#17929
Merged
philipp-spiess merged 8 commits intomainfrom May 9, 2025
Merged
Download platform specific package if optionalDependencies are skipped#17929philipp-spiess merged 8 commits intomainfrom
optionalDependencies are skipped#17929philipp-spiess merged 8 commits intomainfrom
Conversation
45ee6a1 to
ade7619
Compare
ade7619 to
6e16360
Compare
3379069 to
2164265
Compare
RobinMalfait
reviewed
May 9, 2025
|
|
||
| const version = packageJson.version | ||
|
|
||
| function getPlatformPackageName() { |
Member
There was a problem hiding this comment.
I wonder if we can somehow re-use this logic from what NAPI generated for us. Since that is generated and using require already it might not be as easy.
Just have to make sure that if we make changes, that we keep this in sync
Contributor
Author
There was a problem hiding this comment.
I was looking at the napi stuff and it seems to be much more (it generates names for packages that we don't even have etc). I agree that ideally we can reuse it but the function for that is not exported so I'm not sure how. MAYBE we can just try and load the index.js file tbh but then we still don't need which file to load haha
Co-authored-by: Robin Malfait <malfait.robin@gmail.com>
philipp-spiess
commented
May 9, 2025
Co-authored-by: Robin Malfait <malfait.robin@gmail.com>
RobinMalfait
approved these changes
May 9, 2025
2 tasks
philipp-spiess
added a commit
that referenced
this pull request
Oct 19, 2025
This PR effectively reverts #17929. The bug in npm that required it was fixed a couple of months ago and with recent changes to pnpm that requires manually approving all postinstall scripts, this is creating some unnecessary noise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #15806
This PR adds a new
postinstallscript to@tailwindcss/oxidethat will attempt to download the platform-specific optional dependency to avoid issues when the package manager does not do that automatically (see #15806). The implementation for this is fairly simple: The npm package is downloaded from the official npm servers and extracted into thenode_modulesdirectory of the@tailwidncss/oxideinstallation.Test plan
Since we still lack a solid repro of #15806, the way I tested this change was to manually remove all automatically-installed optional dependencies and then running the postinstall script manually. The script then downloads the right version package which makes the import to
@tailwidncss/oxidework. An appropriate integration test was added too.I furthermore also validated that:
pnpm installin the dev setup. This is necessary since doing the initial install won't have any binary dependencies yet so it would download invalid versions from npm (as the version numbers locally refer to the last released version). We can safely bail out here though since this was never an issue with local development.@tailwindcss/oxidelibrary is added unless the issue is detected.[ci-all]