Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Porting an action created this way to ESM #883

Closed
glasser opened this issue Mar 29, 2024 · 9 comments
Closed

Porting an action created this way to ESM #883

glasser opened this issue Mar 29, 2024 · 9 comments
Assignees

Comments

@glasser
Copy link

glasser commented Mar 29, 2024

Has anybody had success adapting an action created with this template to ESM ("type": "module")? I'm finding that some modules I want to use as dependencies are only shipped as ESM these days, but it's not as simple as just adding "type": "module" to package.json. When I do that:

diff --git a/package.json b/package.json
index cbebd22..bd5f1a9 100644
--- a/package.json
+++ b/package.json
@@ -3,6 +3,7 @@
   "description": "GitHub Actions TypeScript template",
   "version": "0.0.0",
   "author": "",
+  "type": "module",
   "private": true,
   "homepage": "https://github.com/actions/typescript-action",
   "repository": {

and run npm run all, the bundler fails with

Error: [tsl] ERROR in /private/tmp/typescript-action/src/index.ts(4,21)
      TS2835: Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean './main.js'?

If I then edit the imports in index.ts and main.ts to add .js to the end:

diff --git a/src/index.ts b/src/index.ts
index b08f970..619751c 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -1,7 +1,7 @@
 /**
  * The entrypoint for the action.
  */
-import { run } from './main'
+import { run } from './main.js'
 
 // eslint-disable-next-line @typescript-eslint/no-floating-promises
 run()
diff --git a/src/main.ts b/src/main.ts
index c804f90..ba7c621 100644
--- a/src/main.ts
+++ b/src/main.ts
@@ -1,5 +1,5 @@
 import * as core from '@actions/core'
-import { wait } from './wait'
+import { wait } from './wait.js'
 
 /**
  * The main function for the action.

I find that eslint fails with

/private/tmp/typescript-action/src/index.ts
  4:21  error  Unable to resolve path to module './main.js'  import/no-unresolved

/private/tmp/typescript-action/src/main.ts
  2:22  error  Unable to resolve path to module './wait.js'  import/no-unresolved

While there's probably a real fix for this, I worked around it by disabling the lint rule:

diff --git a/.github/linters/.eslintrc.yml b/.github/linters/.eslintrc.yml
index f452aba..e13a6fb 100644
--- a/.github/linters/.eslintrc.yml
+++ b/.github/linters/.eslintrc.yml
@@ -41,6 +41,7 @@ rules:
     'eslint-comments/no-unused-disable': 'off',
     'i18n-text/no-en': 'off',
     'import/no-namespace': 'off',
+    'import/no-unresolved': 'off',
     'no-console': 'off',
     'no-unused-vars': 'off',
     'prettier/prettier': 'error',

But now Jest fails:

 PASS  __tests__/wait.test.ts
  wait.ts
    ✓ throws an invalid number (11 ms)
    ✓ waits with a valid number (509 ms)

 FAIL  __tests__/main.test.ts
  ● Test suite failed to run

    Cannot find module './wait.js' from 'src/main.ts'

    Require stack:
      src/main.ts
      __tests__/main.test.ts

      1 | import * as core from '@actions/core'
    > 2 | import { wait } from './wait.js'
        | ^
      3 |
      4 | /**
      5 |  * The main function for the action.

      at Resolver._throwModNotFoundError (node_modules/jest-resolve/build/resolver.js:427:11)
      at Object.require (src/main.ts:2:1)
      at Object.<anonymous> (__tests__/main.test.ts:10:1)

 FAIL  __tests__/index.test.ts
  ● Test suite failed to run

    Cannot find module './wait.js' from 'src/main.ts'

    Require stack:
      src/main.ts
      __tests__/index.test.ts

      1 | import * as core from '@actions/core'
    > 2 | import { wait } from './wait.js'
        | ^
      3 |
      4 | /**
      5 |  * The main function for the action.

      at Resolver._throwModNotFoundError (node_modules/jest-resolve/build/resolver.js:427:11)
      at Object.require (src/main.ts:2:1)
      at Object.<anonymous> (__tests__/index.test.ts:5:1)

and that's where I get stuck.

Has anyone done this successfully?

@ncalteen ncalteen self-assigned this Apr 12, 2024
@ncalteen
Copy link
Collaborator

At some point I am going to probably pull the trigger and either convert this repo to ESM or create a separate template that is ESM...suffice to say I ran into a lot of the same headaches, especially since a number of the @octokit packages are converting to ESM lately (e.g. @octokit/graphql-schema).

Unfortunately all the repos I have that do use TypeScript ESM actions are private, so I put one together quickly as a fork of this repo (https://github.com/ncalteen/typescript-esm-action). If you check the commit diff you can see the following need to be updated.

  • Set "type": "module" in package.json.

  • Add .js extensions to relative imports in src/ and __tests__/

  • Install the TypeScript import resolver for ESLint (this is probably why ESLint was still causing you trouble)

    npm i -D eslint-import-resolver-typescript
  • Add the resolver to .github/linters/eslintrc.yml

    settings:
      import/resolver:
        typescript:
          alwaysTryTypes: true
          project: './.github/linters/tsconfig.json'
  • Install the TypeScript import resolver for Jest (similar to above, without this Jest can't find the relative imports)

    npm i -D ts-jest-resolver
  • Add the resolver to the Jest configuration in package.json

    {
      # ...
      "jest": {
        # ...
        "resolver": "ts-jest-resolver",
        # ...
      },
      # ...
    }

After that, npm run all should run without error. I hope that helps!

@ncalteen
Copy link
Collaborator

ncalteen commented May 2, 2024

Heyo! I'm going to go ahead and close this out now, but if you need anything please feel free to reopen :)

@ncalteen ncalteen closed this as completed May 2, 2024
@karpikpl
Copy link

Hi @ncalteen - thanks for providing the example.
I'm trying to use import { graphql } from '@octokit/graphql'
however when I add that import it's still failing in repo templated from https://github.com/ncalteen/typescript-esm-action

Linting and packaging is passing but JEST is failing

@ncalteen
Copy link
Collaborator

Hey @karpikpl, sorry to hear that! I went down a bit of a rabbit hole on this one and think I found a few fixes. Check out the repo again for the changes!

A few highlights on the changes:

  • Jest needs to be manually imported in tests

    import { jest } from '@jest/globals'
  • Mocks are not automatically hoisted in ESM, so they need to be dynamically imported instead

    // Mock the @actions/core library used in main.ts
    jest.mock('@actions/core', () => {
      const actual: typeof import('@actions/core') = jest.requireActual('@actions/core')
    
      return {
        ...actual,
        getInput: jest.fn(),
        setOutput: jest.fn(),
        setFailed: jest.fn(),
        debug: jest.fn()
      }
    })
    
    // Imports must be done dynamically to allow the mocks to be applied
    // See: https://jestjs.io/docs/ecmascript-modules#module-mocking-in-esm
    const core = await import('@actions/core')
  • Regarding using @octokit/graphql, that should work fine now. The example in this repo uses the built-in graphql method from @octokit/rest.

  • Running Jest with TypeScript ESM modules requires setting the --experimental-vm-options flag. I disable the warning in this repo, but just an FYI since Jest's support is still experimental.

@karpikpl
Copy link

Thanks @ncalteen
I don't know what I'm doing wrong :/ this seems overly complicated

All I want is to call graph from typescript, run tests and eslint - but because of those ESM modules that seems impossible

I tried your recent changes but tests failed - looks like graphql is not getting mocked?
https://github.com/karpikpl/typescript-esm-action-sample/actions/runs/9716566623

@ncalteen
Copy link
Collaborator

Yeah not gonna lie, ESM makes things very challenging. I've been tinkering with this in a similar project and found a few other things to update that seem to help. I updated the other ESM example repo and included an API call using the octokit/graphql client. Hopefully that helps!

@paulRbr
Copy link

paulRbr commented Dec 18, 2024

Hi there @ncalteen thanks for helping on this rabbit hole... It seems your repo is now closed/private https://github.com/ncalteen/typescript-esm-action I'm in the middle of trying to upgrade our github action to ESM and I'm fighting the whole toolchain right now 😞.

If you could share that again it would be great! Thanks a lot

@ncalteen
Copy link
Collaborator

Hey sorry about that! Yes I closed it since I had submitted this PR to this repo. If you pull this repo and check out the ncalteen/esm branch, that will include a more up to date and clean ESM action baseline :)

@paulRbr
Copy link

paulRbr commented Dec 19, 2024

Oh sorry I missed #969. I'll check into it. Thanks a lot 🙇

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants