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

Bun integration for disco-(core/node/web) and server #618

Closed

Conversation

peacefulotter
Copy link
Collaborator

This PR changes the node runtime to Bun.
Bun is fast, heavily maintained, and most importantly for this project, allows to avoid the compilation of disco-core whenever a change is made. Moreover, it provides clear type definitions (@types/bun) and a well made test suite (bun:test)

@tharvik
Copy link
Collaborator

tharvik commented Feb 7, 2024

So, I tried a bit more but bun full support seems too early for disco

  • doesn't really work for monorepo
  • it can't replace tsc (the TypeScript compiler) because of missing .d.ts generation
    • it is needed when publishing a version to NPM, or building a local version
  • I tried to get all the files of discojs-* and the server to be watched but I was unable to do so
    • even with path rewriting, it looks into the dist dir which is populated by tsc

Overall, it's a bit disapointing: it's a cool technology but it has to mature a bit more for theses use cases.

I'll close this PR in favor of simply using #617, which is slowler and less fancy but is fonctional.

@peacefulotter
Copy link
Collaborator Author

  • it indeed does not support monorepo, I'll ask Jarred and the team over on discord for it
  • tsc can be used separately,the usage of bun does not prevent the use of tsc it think. Anyways, someone made a bun plugin for it: https://github.com/wobsoriano/bun-plugin-dts
  • I believe the paths to watch are detected during runtime. Files that are imported and functions that are called are added to a watchlist sort of I think. Could you be more specific with the issue you had and what you did exactly? I had some issues with hot reloading and napi. Also, you can try both --hot and --watch, they work slightly differently

@tharvik
Copy link
Collaborator

tharvik commented Feb 9, 2024

ho, I didn't knew about this plugin. at least, it can help consumer of the lib with types.
the issue at core there is that I want to actually publish the packages to NPM so I need to build the code itself as JS. sadly bun really wants to bundle everything so I'm unable to use it for that (no tree shaking for common deps). so it's only useful for running TypeScript, not build it.

  • I believe the paths to watch are detected during runtime. Files that are imported and functions that are called are added to a watchlist sort of I think. Could you be more specific with the issue you had and what you did exactly? I had some issues with hot reloading and napi. Also, you can try both --hot and --watch, they work slightly differently

I was hoping to improve the server build/run workflow, smth like cd server && npx bun run --watch ./src/run_server.ts that restarts the process when modifying smth in disco-node/src (not dist). even when adding some paths config to tsconfig.json, it doesn't watch theses.

"paths": {
    "@epfml/discojs-core": ["../../discojs/discojs-core/src"],
    "@epfml/discojs-node": ["../../discojs/discojs-node/src"]
}

so I still need to build discojs-{core,node} by hand to restart the process.

I think the main issue here is that I don't really know where bun will be useful for us: is doesn't work as a packager, nor as a auto-builder. the only remaining thing is that I can run files, but that's already taken care of by tsc & node.

@tharvik
Copy link
Collaborator

tharvik commented Feb 13, 2024

after discussion with @peacefulotter, bun doesn't seems to work as well as wanted for our use-cases. anyway, it's a very cool intiative, that will probably work better in some months, let's keep an eye on it, thanks for the work.

@tharvik tharvik closed this Feb 13, 2024
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.

2 participants