- Notifications
You must be signed in to change notification settings - Fork455
feat: Add support for TypeScript scripts#477
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
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 ifactions#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.
kf6kjg commentedJul 8, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
TODO: Should add info and examples to README. |
kf6kjg commentedJul 8, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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. |
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
language
was 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 usingtsx, 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.