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

perf: setting up bun is unnecessary #49

Draft
wants to merge 1 commit into
base: development
Choose a base branch
from
Draft

Conversation

0x4007
Copy link
Member

@0x4007 0x4007 commented Mar 20, 2025

Resolves #50

I didn't test this because I'm not 100% of the syntax, but look how GitHub made their example for instant cold starts.

https://github.com/actions/typescript-action/blob/main/action.yml#L22-L24

@0x4007 0x4007 requested review from whilefoo and gentlementlegen and removed request for whilefoo March 20, 2025 14:56
Copy link
Contributor

Caution

No labels are set.

Copy link
Member

@whilefoo whilefoo left a comment

Choose a reason for hiding this comment

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

https://github.com/ubiquity-whilefoo/daemon-pull-review/actions/runs/13975625604/job/39128551650
Looks like it works, the only downside is that we can't control the version - its v20 currently and current LTS version is v22

@0x4007
Copy link
Member Author

0x4007 commented Mar 20, 2025

https://github.com/ubiquity-whilefoo/daemon-pull-review/actions/runs/13975625604/job/39128551650 Looks like it works, the only downside is that we can't control the version - its v20 currently and current LTS version is v22

Instead of checking out the repo, we should consider a curl <raw git url>dist/index.js | node -e ?

@whilefoo
Copy link
Member

Instead of checking out the repo, we should consider a curl <raw git url>dist/index.js | node -e ?

Usually there are multiple files in the dist folder and I don't think this way will make any faster than with git

@0x4007
Copy link
Member Author

0x4007 commented Mar 20, 2025

The point of ncc is to compile one file

And also to bundle all dependencies of course

@whilefoo
Copy link
Member

I know but there are cases where it generates multiple files: https://github.com/ubiquity-os-marketplace/command-ask/tree/development/dist not sure if it actually uses them but there must be a reason
Also package.json is needed to run it as ESM

@0x4007
Copy link
Member Author

0x4007 commented Mar 21, 2025

Time for r&d

@gentlementlegen
Copy link
Member

The reason for it to generate multiple file, is that it happens when external package also include external code that gets referenced within that external library. We could try using bun compiler once again since a lot of updates have been done, but that was actually what caused it to fail (external references).

Plus, you will have multiple files if you generate sourcemaps either way. And for ESM packages there will be a package.json telling node that it should run this as an ESM file as well.

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.

Cold Start Time Optimization
3 participants