googleapis / google-api-nodejs-client

Google's officially supported Node.js client library for accessing Google APIs. Support for authorization and authentication with OAuth 2.0, API Keys and JWT (Service Tokens) is included.
https://googleapis.dev/nodejs/googleapis/latest/
Apache License 2.0
11.38k stars 1.91k forks source link

build(package): generate esm output with exports #3337

Closed kwonoj closed 4 months ago

kwonoj commented 1 year ago

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

Fixes #3335

This PR attempts to address #3335. Given there are a lot of possible ways to achieve cjs/esm compatible pkg support, I can see this PR is not feasible to this repo's convention and likely can be closed without merge. That's totally fine, however would like to kickstart an initial effort.

Among various approach, this PR nearly mimics recent redux-toolkit's approach (without bundling part) and my personal experiences when I dealt with RxJS (though it's not real esm - https://github.com/ReactiveX/rxjs/blob/master/package.json). Detailed exploration can be found at https://blog.isquaredsoftware.com/2023/08/esm-modernization-lessons/, but in crux this PR does these things

Quick local testing confirmed generated pkg can be imported in node.js with cjs / esm (type:module), also typescript definitions are being resolved. The caveat for type resolution is it falls back to cjs types always, but it won't be a huge concern since most of types should be equivalent between 2 build outputs. Below's manual testing from https://arethetypeswrong.github.io/ how imports are being resolved.

┌───────────────────┬───────────────────────────┬────────────────────────┐
│                   │ "googleapis/package.json" │ "googleapis"           │
├───────────────────┼───────────────────────────┼────────────────────────┤
│ node10            │ 🟢 (JSON)                 │ 🟢                     │
├───────────────────┼───────────────────────────┼────────────────────────┤
│ node16 (from CJS) │ 🟢 (JSON)                 │ 🟢 (CJS)               │
├───────────────────┼───────────────────────────┼────────────────────────┤
│ node16 (from ESM) │ 🟢 (JSON)                 │ 🎭 Masquerading as CJS │
├───────────────────┼───────────────────────────┼────────────────────────┤
│ bundler           │ 🟢 (JSON)                 │ 🟢                     │
└───────────────────┴───────────────────────────┴────────────────────────┘

One last thing to note is about conditional exports: conditional exports limits arbitaray imports (including cjs require) to subpath imports, so if anyone used subpath imports like require('googleapi/somepath/imports') explicitly it'll likely fail. I'm not certain this should be treated as major breaking - since the entrypoints reexports everything and individual api should be resolvable withput subpath - though.

kwonoj commented 1 year ago

I did use these to test resolution locally:

//cjs.cjs

const x = require('googleapis');

console.log(require.resolve('googleapis'));
//esm.mjs
// run with node --experimental-import-meta-resolve esm.mjs

(async () => {
  const googleapi = await import.meta.resolve('googleapis');

  console.log(googleapi)
})();
sofisl commented 4 months ago

We discussed this offline, opted instead to add https://github.com/googleapis/google-api-nodejs-client/blob/ea0b1c809c424495391dac8cb78feb6a95f0772d/package.json#L9