Skip to content

Download platform specific package if optionalDependencies are skipped #17929

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

Merged
merged 8 commits into from
May 9, 2025

Conversation

philipp-spiess
Copy link
Member

@philipp-spiess philipp-spiess commented May 8, 2025

Closes #15806

This PR adds a new postinstall script to @tailwindcss/oxide that 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 the node_modules directory of the @tailwidncss/oxide installation.

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/oxide work. An appropriate integration test was added too.

I furthermore also validated that:

  • This works across CI platforms [ci-all]
  • The postinstall script bails out when running pnpm install in 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.
  • The postinstall script does not do anything when the @tailwindcss/oxide library is added unless the issue is detected.

[ci-all]

@philipp-spiess philipp-spiess force-pushed the fix/add-postinstall-script branch from 45ee6a1 to ade7619 Compare May 8, 2025 15:33
@philipp-spiess philipp-spiess force-pushed the fix/add-postinstall-script branch from ade7619 to 6e16360 Compare May 8, 2025 15:41
@philipp-spiess philipp-spiess force-pushed the fix/add-postinstall-script branch from 3379069 to 2164265 Compare May 8, 2025 16:13
@philipp-spiess philipp-spiess marked this pull request as ready for review May 9, 2025 10:12
@philipp-spiess philipp-spiess requested a review from a team as a code owner May 9, 2025 10:13

const version = packageJson.version

function getPlatformPackageName() {
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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 philipp-spiess merged commit 47bb007 into main May 9, 2025
21 checks passed
@philipp-spiess philipp-spiess deleted the fix/add-postinstall-script branch May 9, 2025 12:55
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.

Can't build on Linux ARM -> cannot find module '@tailwindcss/oxide-linux-arm64-gnu'
2 participants