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

[Proposal] Modernize workflow #406

Open
SunsetTechuila opened this issue Feb 11, 2025 · 10 comments
Open

[Proposal] Modernize workflow #406

SunsetTechuila opened this issue Feb 11, 2025 · 10 comments

Comments

@SunsetTechuila
Copy link
Member

SunsetTechuila commented Feb 11, 2025

  1. Switch from npm to pnpm or from Node to Bun for performance and сonvenience reasons
  2. Remove git pre-commit hook - the main branch is protected, and pushing to it already requires passing tests and linting. Pre-commit hooks are very intrusive in general, and only make development more difficult in this particular case
  3. Switch from tsc to tsup for building extension and tests for performance and сonvenience reasons
  4. Use semantic-release and github actions to fully automate publishing for сonvenience, reliability and safety reasons
  5. [Needs research] Make tests more flexible so they can be easily adapted to test the extension in the browser environment
  6. Maybe enforce Node >=20 for development to update dev dependencies
@SunsetTechuila
Copy link
Member Author

@xuhdev thoughts?

@xuhdev
Copy link
Member

xuhdev commented Feb 11, 2025

I agree with what you propose.

For point 1, I would say we only do pnpm not bun at least for now, because bun is still known to have some minor incompatibilities with Node.js and its support is still behind Node.js. Additionally, requiring contributors to install bun is likely a discouraging factor (while it's easy, many people don't want to install an additional piece of software). For these reasons, I don't think the benefit of bun outweighs its disadvantages for our purposes (the dev part is not performance critical).

What do you think?

@xuhdev
Copy link
Member

xuhdev commented Feb 12, 2025

Should we maybe also pin dependencies for better predictability and reliability? See https://docs.renovatebot.com/dependency-pinning/#so-whats-best

@SunsetTechuila
Copy link
Member Author

I don't think it is necessary, since we use a lockfile and tests in ci will fail if it is outdated

@SunsetTechuila
Copy link
Member Author

bun is still known to have some minor incompatibilities with Node.js and its support is still behind Node.js.

Initially, I proposed it as a package manager only, but now I'm going to see if it's possible to replace Node.js entirely with Bun. The built-in test-runner (can be used for unit tests like api.test.ts) and dependency-patching functionality (see #375 (comment)) would be very useful for this project

@xuhdev
Copy link
Member

xuhdev commented Feb 19, 2025

I don't mind, as long as you think its benefit outweighs its cost.

@SunsetTechuila
Copy link
Member Author

I was sceptical about Bun too, but trust me - once you've tried it, you'll never want to go back to Node

@xuhdev
Copy link
Member

xuhdev commented Feb 19, 2025

I don't doubt about the technical excellence of Bun, I like a lot of their features -- I think its negative sides are mostly non-technical and coming from historical burdens in the industry and the current adoption rate. I don't have a strong opinion in advocating for or against Bun, and I haven't used it in a serious project other than having played with it casually.

Since you are a maintainer, I defer to your decision since you definitely have used and understand Bun more than I do 😃

@SunsetTechuila
Copy link
Member Author

SunsetTechuila commented Feb 20, 2025

Update about moving from tsc for transpiling and bundling: it is not possible to sanely configure tsup to bundle all dependencies expect vscode. The config I ended up with looks like that:

external: ['vscode'],
noExternal: [/^(?!vscode$).+$/],

Atrocious. Not providing a straightforward way to bundle all dependencies is a matter of principle for the developers: 1, 2

Using esbuild directly is anything, but convenient, I have tried that several times and the experience has always been miserable to one degree or another.

But I looked at the docs for Bun's built-in bundler and it seems to be exactly what I'm looking for! Although I'd never used it before as it lacked the features I needed.

As I said:

once you've tried it, you'll never want to go back to Node

@SunsetTechuila
Copy link
Member Author

Bun's built-in bundler and it seems to be exactly what I'm looking for

no, it cannot be used rn oven-sh/bun#5866

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

No branches or pull requests

2 participants