octokit / octokit.js

The all-batteries-included GitHub SDK for Browsers, Node.js, and Deno.
MIT License
6.96k stars 1.02k forks source link

[BUG]: Octokit 4 can't be `import`ed or `require`d #2676

Closed s100 closed 4 months ago

s100 commented 4 months ago

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

github-actions[bot] commented 4 months ago

👋 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 commented 4 months ago

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
wolfy1339 commented 4 months ago

Sorry that is my fault.

I forgot to change the build script

wolfy1339 commented 4 months ago

You should be able to point to the files directly in order to import them

import Octokit from "octokit/dist-src/index.js";
github-actions[bot] commented 4 months ago

:tada: This issue has been resolved in version 4.0.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

gr2m commented 4 months ago

I confirmed that this works now:

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