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

"onSuccess" script runs before dts compile done. #700

Open
Misaka-0x447f opened this issue Aug 26, 2022 · 11 comments · May be fixed by #1024
Open

"onSuccess" script runs before dts compile done. #700

Misaka-0x447f opened this issue Aug 26, 2022 · 11 comments · May be fixed by #1024

Comments

@Misaka-0x447f
Copy link

Misaka-0x447f commented Aug 26, 2022

image

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@egoist
Copy link
Owner

egoist commented Aug 26, 2022

yeah this was initially added to let you run the generated js file, so we didn't wait for the dts build to succeed to call this function.

can you explain why you need this? maybe we can call this function again when the dts build is ready:

onSuccess: ({ isDtsReady }) => {
  // isDtsReady: boolean
}

@Misaka-0x447f
Copy link
Author

yeah this was initially added to let you run the generated js file, so we didn't wait for the dts build to succeed to call this function.

can you explain why you need this? maybe we can call this function again when the dts build is ready:

onSuccess: ({ isDtsReady }) => {
  // isDtsReady: boolean
}

I need let the developer knows compile success and is now waiting for change, but not done without a process exit. Maybe add a hardcoded prompt when everything’s done in watch mode is good enough.

@alii
Copy link

alii commented Mar 12, 2023

can you explain why you need this? maybe we can call this function again when the dts build is ready:

@egoist reasons for needing this functionality in #856

@RichiCoder1
Copy link

^ basically the same reason. Need to rename the types (https://twitter.com/robpalmer2/status/1634702219025981440?s=20). Currently doing tsup && script-that-renames.js, but it'd be really nice to consolidate that. (And/or have it be something tsup does)

@alii
Copy link

alii commented Mar 23, 2023

I see three possible solutions here, I'd love to help/work on a PR if an option is decided

  • Include waiting on tsc before calling onSuccess
  • Have an option called onDtsSuccess
  • Emit .d.cts / .d.mts / .d.ts correctly (which is the reason we need to wait for tsc in the first place

@victor9000
Copy link

victor9000 commented Apr 6, 2023

I'm running into this problem as well while using onSuccess to trigger other tasks. I need it to be called once after everything has completed. I'm currently working around this by deleting the dist dir before a build and using the onSuccess below to verify that both steps have completed. It seems like a bit of a hack though.

--onSuccess 'while [ ! -f  ./dist/index.d.ts ]; do sleep 1; done && bash my-command.sh'

@coopbri
Copy link

coopbri commented Sep 1, 2023

What I ended up doing was running the build without DTS generation, then running a DTS-only build in onSuccess. This solution is also a bit of a hack and loses some concurrency, but it works. From there, you can tap into the lifecycle however you want. Implemented here: https://github.com/animareflection/ui/blob/bd10752060d082e51c2d489974d247f6b3d36944/tsup.config.ts#L46-L53

@brc-dd
Copy link

brc-dd commented Oct 1, 2023

One can also do something like this: (not sure though if it works in all cases)

// tsup.config.ts

import { defineConfig } from 'tsup'

export default defineConfig({
  ...
})

process.on('beforeExit', (code) => {
  if (code === 0) {
    // do something here...
  }
})

@NickBolles
Copy link

We're using onsuccess to call yalc publish for a dev command. We see the JS code get updated in the consuming app, but not the typings until the next build. calling onSuccess again when dts is complete, or having another handler for dts completion would be helpful for this usecase too.

@NickBolles NickBolles linked a pull request Oct 17, 2023 that will close this issue
aryaemami59 added a commit to aryaemami59/tiged that referenced this issue Mar 7, 2024
  - Resolved an issue where CommonJS (CJS) type declaration files were not correctly handling the default export. The solution was to append `export = degit` to the `dist/index.d.ts` file, ensuring compatibility and correct recognition of default exports in CJS modules.
  - This change directly targets the problem outlined in [issue #700 on the tsup repository](egoist/tsup#700), where detailed discussion and context can be found. The modification brings our type declarations in line with expected behaviors, facilitating better interoperability and usage clarity.
  - Additionally, this update brings our type declarations into compliance with the standards set forth by [Are the types wrong](https://github.com/arethetypeswrong/arethetypeswrong.github.io/tree/main/packages/cli).
@its-lee
Copy link

its-lee commented Jun 13, 2024

I'm using this to determine when it's safe for downstream code to locally npm install a package as part of my workflow. As things stand, it only pulls in the non .d.ts code which doesn't really work for Typescript consumers.

@nerdyman
Copy link

I ran into this issue using an ESBuild plugin that generates custom .js and d.ts files at the end of the build. It seems that when clean is enabled it doesn't clear out the d.ts files until the dts stage runs so I had a really weird issue where the .js files were being written but the generated .d.ts files never appeared because the DTS stage was deleting them after the onSuccess fired.

Would be great to have an option to wait for the DTS stage to finish too.

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 a pull request may close this issue.

10 participants