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

Rewrite without TypeScript compilation #339

Open
jthegedus opened this issue Jul 6, 2021 · 9 comments
Open

Rewrite without TypeScript compilation #339

jthegedus opened this issue Jul 6, 2021 · 9 comments
Assignees
Labels
enhancement New feature or request

Comments

@jthegedus
Copy link
Contributor

jthegedus commented Jul 6, 2021

TypeScript compilation and toolchain is adding development overhead, making contributions more difficult. In particular, committing of compiled code is unexpected and confusing for those who have not written GitHub Actions before.

We need to maximise ease of contribution as asdf is a large project and ecosystem.

Typing in JavaScript is still possible using JSDoc - https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html

This method of development has these benefits:

  • remove the compilation step
  • removes the need to commit compiled code
  • reduces the toolchain
  • still gives the benefit of typing and editor hinting.

As no major efforts are currently being made to improve or develop the asdf-actions further, I would like to steward this project forward with this as a first step.

Thoughts?

@jthegedus jthegedus added the enhancement New feature or request label Jul 6, 2021
@jthegedus jthegedus self-assigned this Jul 6, 2021
@Kurt-von-Laven
Copy link

I have generally dealt with this sort of problem by using pre-commit. Here is an example of a hook that automatically compiles via Yarn. Is there a way to perform static type checking when using JSDoc?

@jthegedus
Copy link
Contributor Author

I misunderstood how JS Actions function. The bundling is unavoidable due to the dependencies needing to be bundled into a single JS file.

I would like to explore publishing the compiled scripts to the GitHub Release and pulling them in the root scripts as per this example with Golang binaries, but with the compiled JS scripts: https://full-stack.blend.com/how-we-write-github-actions-in-go.html#small-entrypoint-scripts

@Kurt-von-Laven
Copy link

You are correct that the dependencies need to be bundled for a JavaScript GitHub Action to function properly; we had the same confusion initially.

We have been happy with the approach GitHub Actions recommends in ScribeMD/docker-cache. Blend Engineering hints in the article you linked that they would have gone that route if their team hadn't objected to using JavaScript:

Our approach of using prebuilt binaries is the same idea in spirit as the recommended approach for JavaScript actions. For JavaScript actions, it is recommended to use the ncc compiler to create a single index.js file. With this single file entrypoint, the action just executes that file without any other setup necessary. Since Go is a compiled language, there is no direct equivalent of the “I have some source code and an interpreter” Node.js approach, hence the need for including prebuilt binaries. Interestingly enough, the ncc project lists the Go compiler as one of its motivations, so there must be something there!

We use pre-commit to automate as many steps as possible so contributors don't have to be familiar with our project or GitHub Actions. Dev environment setup takes about 5-10 minutes, and I am confident that investment is quickly paid back many times over by the benefits of running CI pipeline locally, including formatting, building minified JavaScript actions, running tests, auditing dependencies for known security vulnerabilities, automatically checking for copy/paste and spelling violations, and much more. Each local run of our CI takes about 1-2 minutes, and is triggered automatically by Git commits and pushes, which means faster dev cycles, less hassle, and less waiting. We also offer a GitHub Action that optimizes pre-commit performance, so a typical run takes about 5-7 minutes in CI on GitHub-hosted dual-core runners.

@jthegedus
Copy link
Contributor Author

I don't need to be convinced of using pre-commit style tools for these pre-push compilation process.

The thing I am not a fan of is storing compiled (JS bundled or Go) artifacts in the git tree. Whether GitHub recommends it or not, it's not ideal usage of Git. Bundled/Compiled artifacts should be treated like release artifacts. Changing to that type of model would restrict usage purely to the published artifact versions instead of being able to reference git commit SHAs or branches, so there are tradeoffs.

I will be making further updates to this repo shortly with changes to the contribution process with git-hooks etc. But not necessarily to the TS compilation.

@Kurt-von-Laven
Copy link

Yeah, I get that; "not ideal usage of Git" is a great way to put it. I expect many organizations would be unable to use this action if it were to execute downloaded code though. One well-established compromise is only committing the compiled JS to specific release branches that contain nothing else.

For whichever steps you feel pre-commit hooks are appropriate, part of the reason I'm advocating our specific pre-commit toolchain (beyond my obvious biases as its primary author) is that it already has first-class support for asdf, including the optimization proposed in #442.

@jthegedus
Copy link
Contributor Author

I expect many organizations would be unable to use this action if it were to execute downloaded code though.

In my experience companies with those types of restrictions would run a Docker build and push to their internal container registry, instead of referencing a v2 type of version which changes under their noses.

@Kurt-von-Laven
Copy link

That is certainly one option for them. Some are also okay with pinning a commit hash, which tools like Renovate can update automatically without internalizing the dependency.

@jthegedus
Copy link
Contributor Author

Yeah, so I think I will rewrite these without TS, but still need to compile into a single file for publishing. So I might not change the publishing model anytime soon. I was just linking that Golang approach for future purposes.

@airtonix
Copy link

airtonix commented Nov 9, 2023

Having the compiled code here is a compliance requirement for orgs that want to know what they are running.

This is largely why github wants this.

Additionally, you can make use of release-please to automate regenerating the compiled code stored in the repo.

Rewriting it without TS is a pretty big mistake as you can't use JSdoc to typecheck the code in automation.

Not sure why you'd want to reduce quality assurance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants