feat: Add support for TypeScript scripts#477
Draft
kf6kjg wants to merge 3 commits into
Draft
Conversation
`fn.name` was introduced in Node 0.10.0, so this improves test suite maintainability without changing the minimum node version. Also added a test that for certain proves that Promises can be awaited.
Both of the .cts and .mts flavors. Because this action is written in CommonJS both have to compile to CommonJS in order to execute. As it is TypeScript there's already an expectation of some slowness, so I went with the approach of running the script via the node VM module. While a cleaner approach, it has the caveat that root level await in the script doesn't work. That should become available if actions#457 is completed.
We lose the more normal looking export in favor of the `return` statement already traditional to this action, and thus can handle await statements without an ESM conversion. This is a separate commit from the former because I think the next major version of this action should switch to ESM, revert this commit, and use the more standard export notation in all supported languages.
Author
|
TODO: Should add info and examples to README. |
Author
|
Sadness, this isn't working as well as I'd hoped. I'd gone with
Bummer, I get to rewrite - what's the point of TS if you can't import more TS? That's MY primary use case. |
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
1 participant
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.
Both of the .cts and .mts flavors. Because this action is written in CommonJS both have to compile to CommonJS in order to execute.
To handle this a new optional attribute
languagewas added that defaults to the current logic of loading the script as CommonJS. Users can thus opt-in to the TypeScript variants if they wish.My commits each do a separate transform of the codebase. The first is merely a minor cleanup of the existing tests. The second does a more complex implementation of TypeScript support that I personally prefer, but that may have to wait until the next major revision and a conversion to ESM. The third commit alters the second such that it re-uses the existing load-as-function logic after the transpile.
I explored using tsx, but while there's some hacky ways to load scripts into it via data URLs, I liked the pure typescript transpile approach better as I wasn't having to do any hacks or rely on a new library.