Correctly output ES Module

[andykenward]

This doesn't relate to a current open issue. But I've found though out the @octokit npm packages that the exports package entry point isn't being used. Which is the modern alternative to main.

As we are compiling commonJS and ES module versions. We need to add .mjs file extension to the ES module build output files. As this is the standard practice to denote the difference from CommonJS and ES module. When not using "type": "module". Read though NodeJs - Modules Packages for more details.

---

Behavior

Before the change?

• Build output using ESBuild
• Build output for ES module had file extension .js
• Build output for NodeJS was only CommonJS and had a target of node14 when the package.json engine denotes "node": ">= 18" is supported.
• No exports package entry point
• JavaScript traditional function expressions used.

After the change?

• Build output using tsup as this can output tree-shakable code and is powered by ESBuild
• Build output for ES module now has the file extension .mjs.
• Build output for NodeJS is CommonJS & ES Module and the target is node18.
• Build output for Browser is CommonJS & ES Module and the target is es2020.
• exports package entry point added

  exports: {
    types: "./dist-types/index.d.ts",
    browser: {
      import: "./dist-web/index.mjs",
      require: "./dist-web/index.js",
    },
    node: {
      import: "./dist-node/index.mjs",
      require: "./dist-node/index.js",
     },
     default: "./dist-src/index.mjs",
   },

• Use JavaScript arrow function expression when possible. This allows tsup to add comments /* @__PURE__ */ to allow users of the npm package to tree shake within their projects. The usage of arrow functions may break other @octokit packages if the require features of traditional function expressions. Something to be aware of.

Other information

tsup does support generating declaration files for each build format output. But I found that due to bundle: true being set and the src/index.ts entry file only having one export that not all the type definitions would be generated. So I've kept the TypeScript type definition output that was already implemented.

---

Additional info

Pull request checklist

• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)
• Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

• Yes (Please add the Type: Breaking change label)
• No

• Usage of Arrow Functions over Traditional Function expressions.
• Usage .mjs file extension to denote ES Module build output.
• Adding of exports package entry point may break existing projects that are using the commonjs output but NodeJS will now use the ES module version. Edge cases?
• NodeJS target set to node18 instead of node14 might break projects that aren't using Node18 even though the package.json engine is set to "node": ">= 18".

Pull request type

Please add the corresponding label for change this PR introduces:

• Bugfix: Type: Bug
• Feature/model/API additions: Type: Feature
• Updates to docs or samples: Type: Documentation
• Dependencies/code cleanup: Type: Maintenance

---

[wolfy1339]

I appreciate the effort you put into this, but I don't think it's the right thing at this moment.

We just released 2 breaking changes in a short time span.

We are rewriting everything into native ESM + typescript definitions over in the octokit-next.js repo

In the meantime is there anything we can do to make things better that wouldn't result in a breaking change?

[gr2m]

Andy I would recommend to open an issue in general before doing a significant change like this in any project. As @wolfy1339 pointed out, this is not the time and place. If this is blocking you, we already migrated several packages over in https://github.com/octokit/octokit-next.js that you can use, just use the @octokit-next/* npm scope. The efforts are on hold unfortunately due to lack of funding.

[andykenward]

No worries. I've already had to implement something similar for actions/toolkit#1436 .

My main issue is supporting ES modules and tree shaking to reduce my project's bundle size.

I've looked at https://github.com/octokit/octokit-next.js, and using that has already reduced my bundle from 559.0kb to 151.2kb.

Thank you both for responding so quickly. Keep up the great work.

[wolfy1339]

We are moving to output ESM from our repos in the mean time until work on octokit-next.js is finished.

If there are any updates that are still relevant once everything has been updated, feel free to resubmit them