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

feat: require TypeScript module resolution compatible exports map #55

Merged
merged 24 commits into from
Jun 27, 2022

Conversation

n1ru4l
Copy link
Collaborator

@n1ru4l n1ru4l commented Jun 15, 2022

To all reviewers: please give feedback and opinions - DO NOT YET MERGE IT

High-level changes of this PR

  1. Remove the prepack command (which did not do anything important)
  2. Introduce bootstrap command
  3. Add jest resolver config transform
  4. Unify bob build command for single-package repo and mono repo
  5. Alter the output folder structure of the build packages
  6. Generate a new exports map that pleases TypeScript module resolution
  7. Add a new command for checking whether commonjs and ESM exports can be used

Details

1. Remove the prepack command

This command is unnecessary and it's usage can safely be removed from all our packages that use bob.

2. Introduce the bootstrap command

The bootstrap command looks for all package.json files within a repository and alters them by adding the opinionated fields (exports, main, module, type, and publishConfig). A full list of the opinionated package.json fields can be found within the src/commands/bootstrap.ts file. In a future release we want to allow customizing the exports map on a package basis.

Furthermore, the bootstrap command looks for all *.ts files and transforms the imports and export paths of RELATIVE packages (./, ../) and adds the .js extension at the end. This is necessary for pleasing the TypeScript compiler, but also ESM resolution to work properly.

Before:

export { createPubSub, PubSub } from './createPubSub';
export type { PubSubEventTarget, PubSubEvent } from './createPubSub';
export { map } from './operator/map';
export { filter } from './operator/filter';
export { pipe } from './utils/pipe';

After:

export { createPubSub, PubSub } from './createPubSub.js';
export type { PubSubEventTarget, PubSubEvent } from './createPubSub.js';
export { map } from './operator/map.js';
export { filter } from './operator/filter.js';
export { pipe } from './utils/pipe.js';

NOTE: We need an eslint rule in each package that every relative import ends with .js. The problem is that we could enforce it via typescript compiler when using "moduleResolution": "Node16". However, it will break our build as w have a lot of dev-only dependencies that then would raise compiler errors in test cases. We can't really run jest in ESM just yet (as it is a nightmare). UPDATE: The script for checking CommonJS and ESM (7.) imports actually already covers this.

So for all the projects upgrading to the latest bob should be as simple as:

  1. install new bob
  2. run bob bootstrap
  3. that is it (but actually not see next step 🥲 )

3. jest-resolver config

Unfortunately, adding .js to relative imports (as mentioned in #2) breaks jest module resolution in commonjs mode. For now, we should not upgrade to running tests on ESM as it is a mess and requires using experimental node features.

Fortunately, we can hook into the jest resolution algorithm. In order to tackle our ongoing DRY dev tooling effort the jest-resolver.js file lives within bob. That means any project using bob can easily reference it in the jest.config.js:

module.exports = {
  testEnvironment: 'node',
  rootDir: ROOT_DIR,
  // .... blabla other fields

  // vvvvvvvvvv THIS IS THE IMPORTANT PART
  resolver: 'bob-the-bundler/jest-resolver.js',
}

In the future, I wanna experiment with whether we can move more of the shared configuration into bob itself.

4. Unify bob build command for single-package repo and mono repo

Previously, bob build did completely different things for single-package repos and mono repo. I altered the implementation to share as much logic as possible, which will make future improvements/refinements easier. We now also have an integration test suite that we can improve over time. It is now no longer necessary to run tsc separately before running bob build. bob build now takes care of everything. You just have to run bob build. That is it.

5. Alter the output folder structure of the build packages

Old structure

package.json
index.mjs
index.js
index.d.ts
foo.d.ts

New structure

package.json
esm/index.js
esm/foo.js
cjs/package.json
cjs/index.js
cjs/foo.js
typings/index.d.ts
typings/foo.d.ts

We no longer bundle the code into a single flat file.

We use the .js extension everywhere (we don't need .mjs or .cjs)

The package.json contains {"type":"module"} for instructing the Node.js resolution algorithm to treat all files within this folder as esm.

The cjs/package.json contains {type:"commonjs"} for instructing the Node.js resolution algorithm to treat all files within this sub folder as commonjs.

Fortunately, we can use the same typings for TypeScripts ESM and CommonJS support.

6. Generate a new exports map that pleases TypeScript module resolution

In order to pass the TypeScript compiler options for ECMA modules, the exports map must be altered to include a "typings" key.

TypeScript compiler options:

"module": "Node16",
"moduleResolution": "Node16",

More information about the TypeScript Exports Map: https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#package-json-exports-imports-and-self-referencing

Legit exports map example:

{
  "exports": {
    ".": {
      "require": {
        "types": "./dist/typings/index.d.ts",
        "default": "./dist/cjs/index.js"
      },
      "import": {
        "types": "./dist/typings/index.d.ts",
        "default": "./dist/esm/index.js"
      },
      "default": {
        "types": "./dist/typings/index.d.ts",
        "default": "./dist/esm/index.js"
      }
    },
    "./*": {
      "require": {
        "types": "./dist/typings/*.d.ts",
        "default": "./dist/cjs/*.js"
      },
      "import": {
        "types": "./dist/typings/*.d.ts",
        "default": "./dist/esm/*.js"
      },
      "default": {
        "types": "./dist/typings/*.d.ts",
        "default": "./dist/esm/*.js"
      }
    },
    "./package.json": "./package.json"
  }
}

7. Add a new command for checking whether commonjs and ESM exports can be used

We have a few test-esm.js script that runs on CI for checking whether the ESM files can be imported. bob check replaces this script and furthermore also checks the exports map including CJS and ESM, by spawning node processes for each file.

Upgrading

The following PRs are example upgrades:

Envelop: n1ru4l/envelop#1426 (comment)
GraphQL Yoga: dotansimha/graphql-yoga#1307
graphql-tools: ardatan/graphql-tools#4539

The current upgrade path:

  1. add bob canary build
  2. run bob bootstrap
  3. remove "prebuild": "bob prepack" from all package.json files
  4. delete tsconfig.build.json
  5. replace tsc && bob build with bob build
  6. Add resolver: 'bob-the-bundler/jest-resolver.js', to jest.config.js
  7. Replace test-esm.js script with bob check on CI tasks

Pending TODOS:


Closes #48
Closes #39
Closes #47
Closes #71

@changeset-bot
Copy link

changeset-bot bot commented Jun 15, 2022

🦋 Changeset detected

Latest commit: c2c8cf1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
bob-the-bundler Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@n1ru4l n1ru4l changed the title Feat typescript module resolution feat: require TypeScript module resolution compatible exports map Jun 15, 2022
@theguild-bot
Copy link

theguild-bot commented Jun 15, 2022

The latest changes of this PR are not available as alpha, since there are no linked changesets for this PR.

@n1ru4l n1ru4l force-pushed the feat-typescript-module-resolution branch from 8cf63c9 to eb984f2 Compare June 20, 2022 06:13
@n1ru4l n1ru4l force-pushed the feat-typescript-module-resolution branch from 2e18f50 to cceffca Compare June 20, 2022 10:22
@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Jun 21, 2022

This is now tested in dotansimha/graphql-yoga#1307 with feedback being collected in dotansimha/graphql-yoga#1217

@n1ru4l

This comment was marked as resolved.

@n1ru4l n1ru4l force-pushed the feat-typescript-module-resolution branch 4 times, most recently from bdaa67c to c478235 Compare June 22, 2022 06:53
@n1ru4l n1ru4l force-pushed the feat-typescript-module-resolution branch 2 times, most recently from 8145794 to 6d0c3d3 Compare June 22, 2022 17:49
@n1ru4l n1ru4l force-pushed the feat-typescript-module-resolution branch from 6d0c3d3 to 18f8755 Compare June 22, 2022 17:50
@n1ru4l n1ru4l force-pushed the feat-typescript-module-resolution branch from 18f8755 to 47c190f Compare June 22, 2022 17:56
@n1ru4l n1ru4l requested review from ardatan, kamilkisiela and dotansimha and removed request for ardatan June 23, 2022 07:38
@dotansimha
Copy link
Collaborator

@n1ru4l I love it!!
regarding bootstrap command, do we have different flavors for different package types? (CLI/lib) or the config is the same for all?

@n1ru4l n1ru4l force-pushed the feat-typescript-module-resolution branch from bc31a81 to 55ec258 Compare June 23, 2022 14:53
@n1ru4l n1ru4l force-pushed the feat-typescript-module-resolution branch from 9da2e6f to b3c06ea Compare June 24, 2022 10:11
@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Jun 27, 2022

@n1ru4l I love it!! regarding bootstrap command, do we have different flavors for different package types? (CLI/lib) or the config is the same for all?

@dotansimha Right now the bootstrap command does not care, we can make it more flexible in a follow-up PR. I also think we should have some kind of command for easily adding a new package to a monorepo or converting a single package repo into a monorepo.

@n1ru4l n1ru4l merged commit ae0b4b2 into master Jun 27, 2022
@n1ru4l n1ru4l deleted the feat-typescript-module-resolution branch June 27, 2022 09:36
@charlypoly
Copy link

@n1ru4l used it on https://github.com/charlypoly/graphql-yoga-nestjs, worked like a charm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants