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 napi double .d.ts formatting #9238

Merged
merged 1 commit into from
Oct 9, 2024
Merged

fix napi double .d.ts formatting #9238

merged 1 commit into from
Oct 9, 2024

Conversation

dimitropoulos
Copy link
Contributor

Description

napi-rs generates a .d.ts file for us in turbo-repository. Unfortunately, this file doesn't match our project's formatting (the generated file doesn't include semicolons). This means that every time we locally run a command (for example, pnpm test) that courses through this generation we jiggle this file, creating a lot of noise when developing.

This small PR fixes this by simplifying the script and running prettier on it.

Testing Instructions

run pnpm build from packages/turbo-repository before this PR and see the file change, and notice that after this PR the file change goes away.

Copy link

vercel bot commented Oct 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 9, 2024 4:09pm
8 Skipped Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Oct 9, 2024 4:09pm
examples-designsystem-docs ⬜️ Ignored (Inspect) Oct 9, 2024 4:09pm
examples-gatsby-web ⬜️ Ignored (Inspect) Oct 9, 2024 4:09pm
examples-kitchensink-blog ⬜️ Ignored (Inspect) Oct 9, 2024 4:09pm
examples-native-web ⬜️ Ignored (Inspect) Oct 9, 2024 4:09pm
examples-svelte-web ⬜️ Ignored (Inspect) Oct 9, 2024 4:09pm
examples-tailwind-web ⬜️ Ignored (Inspect) Oct 9, 2024 4:09pm
examples-vite-web ⬜️ Ignored (Inspect) Oct 9, 2024 4:09pm


# Unfortunately, when napi generates a .d.ts file, it doesn't match our formatting rules (it doesn't have semicolons).
# Since there's now way to configure this from napi itself, so we need to run prettier on it after generating it.
node_modules/.bin/prettier --write js/index.d.ts
Copy link
Contributor Author

@dimitropoulos dimitropoulos Oct 9, 2024

Choose a reason for hiding this comment

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

we could really min/max this by switching the script around a bit and only running prettier when not in release mode (we don't generate the file in release mode for some reason), but I thought it'd be simpler to leave it as-is and just always format.

@@ -5,8 +5,8 @@
"bugs": "https:/vercel/turborepo/issues",
"homepage": "https://turbo.build/repo",
"scripts": {
"build": "napi build --platform -p turborepo-napi --cargo-cwd ../../ --cargo-name turborepo_napi native --js false --dts ../js/index.d.ts && mkdir -p js/dist && cp js/index.js js/dist/index.js && cp js/index.d.ts js/dist/index.d.ts",
"build:release": "napi build --release --platform -p turborepo-napi --cargo-cwd ../../ --cargo-name turborepo_napi native --js false",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

was it intentional that we, previously, didn't generate the dts file when in release mode?

Copy link
Member

Choose a reason for hiding this comment

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

Probably not, the release process for this package is far more ad-hoc than the primary turbo release workflow.

@dimitropoulos dimitropoulos merged commit a72f7d5 into main Oct 9, 2024
43 checks passed
@dimitropoulos dimitropoulos deleted the dimitri/napi-dts branch October 9, 2024 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants