refactor: convert action to TS and bundle code#95
Merged
dsherret merged 6 commits intodenoland:mainfrom May 12, 2025
Merged
Conversation
fa22023 to
40ef822
Compare
40ef822 to
663669c
Compare
Contributor
Author
|
@crowlKats Sorry for pinging directly, I've tried the |
dsherret
approved these changes
May 12, 2025
Contributor
There was a problem hiding this comment.
LGTM. Thanks!
I'm going to merge this in a bit. I just need to investigate this Deno bug with the frozen lockfile (opened denoland/deno#29265)
Contributor
Author
|
Awesome, thanks for reviewing and merging this one @dsherret! |
crowlKats
pushed a commit
that referenced
this pull request
May 15, 2025
(cherry picked from commit 95bbb87)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In #89 I tried adding automatic caching of installed dependencies for Deno. But that PR makes the
node_modulesexplode in size if added to Git, due to@actions/cacherequiring a lot of subdependencies.This PR adds bundling of the action code, which will reduce the impact by a lot (see below), since
node_moduleswill no longer be cloned. If this PR is merged, it should make the remaining work on #89 trivial.There's mostly pro's to using the approach in this PR (which works similarly to
@actions/checkoutand other GitHub created Actions), except that contributors to this action needs to rundeno task buildprior to committing any changes (or checks will fail in PRs).Commits
.vscodefolder with setting enabling deno, formatting and lintingdist/Action size
The amount of files cloned when action is checked out,
mainvs this PR:Before
node_modules)node_modules)After
So bundling code should make it much quicker to clone and run actions, due to smaller amount of files to for Git to handle, as well as a single file being much quicker to run in Node.js over hundreds.
Bundler choice
I landed at using
tsdown, which is usingrolldownunder the hood. Things I tried that did not work well:@deno/dnt: A had a lot of errors when I tried, and this also produces a lot of unnecessary files for this use-case (package.json, types, etc)esbuild+jsr:@luca/esbuild-deno-loader: This worked to produceCJSoutput, but trying to produceESMdid not go well. Though this approach is very nice in avoiding an install step and downloading/caching dependencies during bundling.tsdown+deno-vite-plugin: I tried to hack around making the vite plugin work (callingconfigResolved()manually), but had various issues and could not get this working in the end.Finally, I gave up and used
tsdownafter just installingnode_moduleswith Deno first. This worked well on the first try, and was quicker than the other options anyway. It also seems to work withESMoutput without issues.