Description
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 script body using an monkey-patched require(...) 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 import keyword, or a dynamic import(...) method call. 💚
// Syntax-sugared import statements
import { has } from 'lodash-es' // An ESM package
import { getOctokit } from '@actions/github' // A CommonJS package
// Dynamic import method call
const { setOutput } = await import('@actions/core') // A CommonJS packageThe 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 the require(...) approach. The import keyword is not available within CommonJS files but it is possible to use the dynamic import(...) approach instead. However, the import method is not currently accessible within github-script blocks — I tried that and it failed! ❌
Unfortunately, this means that I can no longer utilize this Action's handy require(...) behavior on my own application's local modules as we recently converted to ESM (after becoming blocked by a dependency that converted to ESM 😬), e.g. github/docs@.github/workflows/staging-deploy-pr.yml#L52-L61 😭
Describe 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 import keyword 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-script Action, which involves using the esm npm 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-script and just run a roughly equivalent Node.js script that uses @actions/core, etc. 😢
Additional context
N/A 🤷🏻♂️