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
Support importing ESM files #168
Comments
|
This issue is stale because it has been open for 60 days with no activity. Remove the "Stale" label or comment on the issue, or it will be closed in 7 days. |
|
commenting to prevent auto-removal. Will this be resolved when node 16 goes LTS? (not sure what github actions will be doing with node on Ubunut runners) |
|
@LongLiveCHIEF That's a good point, having Node.js 16.x in place on the runners would probably be the first step necessary. After that, there might still be more work (if possible? |
|
It's not an issue of the runner image, but the runner itself. The runner bundles its own node because you may have a random version by default (because you ran We'll add node16 as an option |
|
This issue is stale because it has been open for 60 days with no activity. Remove the "Stale" label or comment on the issue, or it will be closed in 7 days. |
This would be awesome but it may be wishful thinking. |
|
JavaScript Actions can now run using Node.js 16 by changing the I imagine this means we can now investigate if switching to |
|
With #235 and https://github.com/joshmgross/actions-testing/runs/5130290014?check_suite_focus=true Workflow: - uses: thboop/github-script@d2ed94b14f4e194b2afdd7fe2af49e0d93c42d92
with:
script: |
try {
console.log('Before dynamic import')
const { default: printStuff } = await import('${{ github.workspace }}/src/print-stuff.js')
console.log('After dynamic import')
await printStuff()
console.log('At the end')
} catch (error) {
console.error('Threw an error!')
throw error
}
{
"name": "",
"description": "",
"type": "module",
"private": true,
"version": "0.1.0",
"dependencies": {},
"devDependencies": {}
}
export default function printStuff() { console.log('stuff') }Since we're not wrapping Thanks @JamesMGreene for all the help |
Is your feature request related to a problem? Please describe.
This Action recently added support for importing CommonJS-based npm packages and local modules within the🎉
scriptbody using an monkey-patchedrequire(...)method. That is awesome!Alas, the Node.js community has been moving away from CommonJS more and more now that the ECMAScript Modules (ESM) standard is out of its experimental status.
The good news? If your module/application is written with ESM, it can import both other ESM files and CommonJS modules using the new💚
importkeyword, or a dynamicimport(...)method call.The bad news? If your module/application is written with CommonJS, the❌
require(...)method can only import other CommonJS modules. It is not possible to directly import ESM files because they are not guaranteed to be synchronous, which is a requirement of therequire(...)approach. Theimportkeyword is not available within CommonJS files but it is possible to use the dynamicimport(...)approach instead. However, theimportmethod is not currently accessible withingithub-scriptblocks — I tried that and it failed!Unfortunately, this means that I can no longer utilize this Action's handy😬 ), e.g. 😭
require(...)behavior on my own application's local modules as we recently converted to ESM (after becoming blocked by a dependency that converted to ESMgithub/docs@.github/workflows/staging-deploy-pr.yml#L52-L61Describe the solution you'd like
In an ideal world, I think we probably should have only added support for the ESM approach to this module such that we could've supported both. Alas, timing and knowledge such as they were, we opted for the standard CommonJS
require(...)approach.Unfortunately, this would prevent us from adding full support for ESM's
importkeyword now, or at least not without at least bumping this Action's major version number to indicate the breaking change.As such, I think a more reasonable compromise would be to just add support for the dynamic
import(...)method calls within the current wrapping function.Describe alternatives you've considered
I did some fairly extensive testing of a handful of approaches and potential workarounds to this situation:
You can see the implementation details in this workflow file.
The failed job above ☝🏻 that I would like to see passing with the proposed solution is the "Try ESM dynamic imports" (
esm-dynamic-imports) job.Alternative (1):
I was only able to get 1 of the 6 attempts working with the current state of the
github-scriptAction, which involves using theesmnpm package and some questionable additional workarounds that may or may not work when applied to more advanced ESM dependency chains. 🤷🏻♂️Alternative (2):
Not use😢
actions/github-scriptand just run a roughly equivalent Node.js script that uses@actions/core, etc.Additional context
N/A 🤷🏻♂️
The text was updated successfully, but these errors were encountered: