Skip to content
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

[BUG]: Octokit 4 can't be imported or required #2676

Closed
1 task done
s100 opened this issue May 8, 2024 · 8 comments · Fixed by #2678
Closed
1 task done

[BUG]: Octokit 4 can't be imported or required #2676

s100 opened this issue May 8, 2024 · 8 comments · Fixed by #2678
Labels
released Type: Bug Something isn't working as documented

Comments

@s100
Copy link

s100 commented May 8, 2024

What happened?

This should work:

a.mjs:

import * as octokit from 'octokit'

However this fails with a module loading error (see later).

Note that octokit@4.0.1 ships with "type": "module" in its package.json, indicating a pure ESM package, but "main" points to dist-node/index.js, which is clearly a CJS module making use of module.exports and require. Something's gone wrong here.

Versions

Octokit 4.0.1, Node.js 18.17.1

Relevant log output

C:\...>node a.mjs
file:///C:/.../node_modules/octokit/dist-node/index.js:29
module.exports = __toCommonJS(dist_src_exports);
^

ReferenceError: module is not defined in ES module scope
    at file:///C:/.../node_modules/octokit/dist-node/index.js:29:1
    at ModuleJob.run (node:internal/modules/esm/module_job:194:25)

Node.js v18.17.1

Code of Conduct

  • I agree to follow this project's Code of Conduct
@s100 s100 added Status: Triage This is being looked at and prioritized Type: Bug Something isn't working as documented labels May 8, 2024
@github-project-automation github-project-automation bot moved this to 🆕 Triage in 🧰 Octokit Active May 8, 2024
Copy link

github-actions bot commented May 8, 2024

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

@s100
Copy link
Author

s100 commented May 8, 2024

I'm finding that CJS can't use Octokit 4 either:

a.js:

require('octokit')
C:\...>node a.js
C:\...\a.js:1
require('octokit')
^

Error [ERR_REQUIRE_ESM]: require() of ES Module C:\...\node_modules\octokit\dist-node\index.js from C:\...\a.js not supported.
index.js is treated as an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which declares all .js files in that package scope as ES modules.
Instead rename index.js to end in .cjs, change the requiring code to use dynamic import() which is available in all CommonJS modules, or change "type": "module" to "type": "commonjs" in C:\...\node_modules\octokit\package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead).

    at Object.<anonymous> (C:\...\a.js:1:1) {
  code: 'ERR_REQUIRE_ESM'
}

Node.js v18.17.1

@s100 s100 changed the title [BUG]: Octokit 4 can't be imported [BUG]: Octokit 4 can't be imported or required May 8, 2024
@klippx klippx mentioned this issue May 8, 2024
1 task
@wolfy1339
Copy link
Member

Sorry that is my fault.

I forgot to change the build script

wolfy1339 added a commit that referenced this issue May 8, 2024
@wolfy1339

This comment was marked as outdated.

@wolfy1339 wolfy1339 removed the Status: Triage This is being looked at and prioritized label May 8, 2024
@gr2m gr2m closed this as completed in #2678 May 8, 2024
@github-project-automation github-project-automation bot moved this from 🆕 Triage to ✅ Done in 🧰 Octokit Active May 8, 2024
Copy link

github-actions bot commented May 8, 2024

🎉 This issue has been resolved in version 4.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gr2m
Copy link
Contributor

gr2m commented May 8, 2024

I confirmed that this works now:

// test.mjs
import { Octokit } from "octokit";
console.log(Octokit.VERSION);
// 6.1.2

@rolwin100
Copy link

I'm still facing the issue with nestjs. I solved it by downgrading it to version 3. This solved my issue

@wolfy1339
Copy link
Member

NestJS can't do ESM only packages.

There is a new NodeJS feature that can allow you to use require() in order to import ESM packages.
However support is limited to 23 currently, and is going to be backported to 22 soon.

You can also use await import().

If you are stuck on a version of Node that is older, and require CJS stick to v3 of Octokit.
We will support bug fixes for it for the time being

Please open a new issue with proper information if ever you encounter an issue like this again

@octokit octokit locked and limited conversation to collaborators Oct 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
released Type: Bug Something isn't working as documented
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants