Load codegen.ts in ESM projects #9086

beerose · 2023-03-01T14:06:48Z

Follow up PR: #9094

Description

This PR handles loading codegen.ts in projects using ESM. Some of the approaches I tried:

Changing cosmiconfig's configuration (nothing relevant that would make this work out of the box)
Registering TS with ts-node (I tried different configurations to be able to use await import)
Using eval: eval("import("pathname")"

I originally went with execSync(tsc ...) to compile the config file to CommonJS and cache the results. But the caching part has a bunch of edge cases so I took a different approach and used jiti to load the file.

See benchmarks ⏳

ESM path — tsc, cache miss:

➜  vite-react-ts git:(vite-configts-fix) ✗ time yarn codegen  
yarn run v1.22.19
$ graphql-codegen --config codegen.ts
✔ Parse Configuration
✔ Generate outputs
✨  Done in 7.46s.
yarn codegen  7.81s user 0.69s system 111% cpu 7.649 total

ESM path — tsc, cache hit:

➜  vite-react-ts git:(vite-configts-fix) ✗ time yarn codegen
yarn run v1.22.19
$ graphql-codegen --config codegen.ts
✔ Parse Configuration
✔ Generate outputs
✨  Done in 3.69s.
yarn codegen  3.22s user 0.38s system 92% cpu 3.896 total

ESM path — jiti:

➜  vite-react-ts git:(vite-configts-fix) ✗ time yarn codegen
yarn run v1.22.19
$ graphql-codegen --config codegen.ts
✔ Parse Configuration
✔ Generate outputs
✨  Done in 3.91s.
yarn codegen  3.33s user 0.36s system 90% cpu 4.094 total

.cts — jiti:

➜  vite-react-cts git:(vite-configts-fix) ✗ time yarn codegen
yarn run v1.22.19
$ graphql-codegen --config codegen.cts
✔ Parse Configuration
✔ Generate outputs
✨  Done in 3.36s.
yarn codegen  3.21s user 0.34s system 99% cpu 3.558 total

Non-ESM path:

➜  apollo-client git:(vite-configts-fix) ✗ time yarn codegen      
yarn run v1.22.19
$ graphql-codegen --config codegen.ts
✔ Parse Configuration
✔ Generate outputs
✨  Done in 3.92s.
yarn codegen  3.22s user 0.38s system 87% cpu 4.116 total

Some of the next steps are:

Make the same change graphql-config as well (same issue) — PR: Support ESM and .mts/.cts extensions with jiti loader kamilkisiela/graphql-config#1286

Related Closes #8509, Closes #8862, Closes #8809

Type of change

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Created a new example based on reproductions steps in the issue

Test Environment:

OS: macOS
NodeJS: 19.x

Checklist:

I have followed the CONTRIBUTING doc and the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

changeset-bot · 2023-03-01T14:06:52Z

🦋 Changeset detected

Latest commit: f010b06

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

This PR includes changesets to release 2 packages

Name	Type
@graphql-codegen/cli	Patch
@graphql-cli/codegen	Patch

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

github-actions · 2023-03-01T14:43:00Z

🚀 Snapshot Release (`alpha`)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package	Version	Info
`@graphql-cli/codegen`	`3.0.4-alpha-20230307110723-f010b0664`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/cli`	`3.2.2-alpha-20230307110723-f010b0664`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/visitor-plugin-common`	`3.0.2-alpha-20230307110723-f010b0664`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/typescript-document-nodes`	`3.0.2-alpha-20230307110723-f010b0664`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/gql-tag-operations`	`2.0.2-alpha-20230307110723-f010b0664`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/typescript-operations`	`3.0.2-alpha-20230307110723-f010b0664`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/typescript-resolvers`	`3.1.1-alpha-20230307110723-f010b0664`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/typed-document-node`	`3.0.2-alpha-20230307110723-f010b0664`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/typescript`	`3.0.2-alpha-20230307110723-f010b0664`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/client-preset`	`2.1.1-alpha-20230307110723-f010b0664`	npm ↗︎ unpkg ↗︎
`@graphql-codegen/graphql-modules-preset`	`3.1.1-alpha-20230307110723-f010b0664`	npm ↗︎ unpkg ↗︎

github-actions · 2023-03-01T14:49:04Z

💻 Website Preview

The latest changes are available as preview in: https://5d1dbded.graphql-code-generator.pages.dev

enisdenjo

Your code is very clean and easy to read - this looks great!

About the ESM detection, maybe I am too naive, but couldn't we just look for the import keyword in the config? We can use a regex for increased accuracy, something like import (.* from|"|').

Also, a test case for this would be great.

beerose · 2023-03-02T11:00:23Z

About the ESM detection, maybe I am too naive, but couldn't we just look for the import keyword in the config? We can use a regex for increased accuracy, something like import (.* from|"|').

Hmm, I'm not sure. You can have a TypeScript file with ESM syntax that technically is a CommonJS and can be required by ts-node. Notice that this bug only occurred with "type": "module" in package.json, and the workaround was to use .cjs instead of .ts for the config — link.

packages/graphql-codegen-cli/src/config.ts

enisdenjo · 2023-03-02T12:24:06Z

Yes, you're right. But, it might make sense to immediately drop to tsc even on a hint, mainly for performance reasons (even if it would've worked with ts-node, it would with tsc too).

In future steps we might even drop ts-node altogether and programmatically use tsc with transpileModule.

packages/graphql-codegen-cli/package.json

enisdenjo

This looks great to me! I like that we dropped the cosmiconfig-typescript-loader especially.

We can go ahead and merge after the CI's all green.

packages/graphql-codegen-cli/package.json

dimaMachina · 2023-03-06T16:01:30Z

packages/graphql-codegen-cli/src/config.ts

-      // #8437: conflict with `graphql-config` also using TypeScriptLoader(), causing a double `ts-node` register.
-      const tsLoader = TypeScriptLoader({ transpileOnly: true });
-      return tsLoader(filepath, content);
+      const jiti = require('jiti')(__filename, {


You can use import

Also do we bundle codegen cli to both cjs and Esm? If yes __filename will be not exist in esm

Ah right. We do bundle to both. What's the best way to deal with it? Something like this:

import { fileURLToPath } from 'node:url'; const __filename = fileURLToPath(import.meta.url)

?

@enisdenjo could we add banner with bob? Or should we bundle to Esm only instead?

@beerose yeah but we need to add it only to ESM build 😅

@saihaj, do you have any thoughts on that?

I think script is fine for simplicity. We do similar thing too for binary. Ideally bob should have some hooks that we can use to call scripts as needed.

should we do the same thing here?

if there is no other workaround..

Ok, so it looks like we can workaround it by not providing __filename at all — jiti resolves it to process.cwd(), which I guess it's correct? https://github.com/unjs/jiti/blob/main/src/jiti.ts#L101

Well, the CI passes 😅

dimaMachina · 2023-03-06T16:07:57Z

This looks great to me! I like that we dropped the cosmiconfig-typescript-loader especially.

We can go ahead and merge after the CI's all green.

@enisdenjo I think we can remove ts-node too now?

enisdenjo · 2023-03-06T16:30:48Z

I think we can remove ts-node too now?

Yes sir! I think it was only necessary for cosmiconf.

beerose · 2023-03-06T16:35:37Z

I think we can remove ts-node too now?

Yes sir! I think it was only necessary for cosmiconf.

Is there any documentation that needs to be updated regarding this?

saihaj · 2023-03-06T16:43:42Z

I think we can remove ts-node too now?

Yes sir! I think it was only necessary for cosmiconf.

Is there any documentation that needs to be updated regarding this?

I think we are good here. ts-node already caused issues #8809 so I suppose now we also fixed issues for yarn pnp users

beerose added 4 commits March 1, 2023 10:48

Add failing test

b777bae

Compile config file for esm projects

be7b550

Move comment

f5490b7

Make sure example app works; add tests

0577f5a

Merge branch 'master' into vite-configts-fix

9d5c25d

beerose and others added 5 commits March 1, 2023 21:10

Add TypeScript to optional peer deps

f9bc89f

Add caching for transpiled files

7023201

chore(dependencies): updated changesets for modified dependencies

189cfbb

Simplify code a bit

b34166c

Add changeset

48dfe8b

beerose marked this pull request as ready for review March 2, 2023 09:55

beerose requested review from n1ru4l, dimaMachina and charlypoly March 2, 2023 10:01

enisdenjo reviewed Mar 2, 2023

View reviewed changes

beerose commented Mar 2, 2023

View reviewed changes

packages/graphql-codegen-cli/src/config.ts Outdated Show resolved Hide resolved

Support .mts and .cts config extensions

56635af

This was referenced Mar 2, 2023

Support ESM and .mts/.cts config extensions kamilkisiela/graphql-config#1281

Merged

Support .mts and .cts config extensions #9094

Merged

beerose and others added 4 commits March 3, 2023 17:40

Use jiti insetad of custom cache and tsc

c6140ac

Merge branch 'mts-cts-support' into vite-configts-fix

3ffddaf

Support mts and cts with jiti

1b08466

chore(dependencies): updated changesets for modified dependencies

332b0b6

beerose mentioned this pull request Mar 3, 2023

Support ESM and .mts/.cts extensions with jiti loader kamilkisiela/graphql-config#1286

Merged

9 tasks

Update graphql-config

386b521

Merge branch 'master' into vite-configts-fix

1dee0a7

dimaMachina reviewed Mar 6, 2023

View reviewed changes

packages/graphql-codegen-cli/package.json Show resolved Hide resolved

enisdenjo approved these changes Mar 6, 2023

View reviewed changes

enisdenjo reviewed Mar 6, 2023

View reviewed changes

packages/graphql-codegen-cli/package.json Outdated Show resolved Hide resolved

dimaMachina reviewed Mar 6, 2023

View reviewed changes

beerose and others added 3 commits March 6, 2023 17:08

Use import over require

20183f0

chore(dependencies): updated changesets for modified dependencies

e58d8b4

Merge branch 'master' into vite-configts-fix

276130e

Remove ts-node from graphql-codegen-cli

8206bae

chore(dependencies): updated changesets for modified dependencies

2b36f09

saihaj approved these changes Mar 6, 2023

View reviewed changes

beerose force-pushed the vite-configts-fix branch from 3dbaac7 to 2b36f09 Compare March 6, 2023 17:16

beerose added 2 commits March 6, 2023 20:20

Pass empty string to jiti

1ee2ad4

Merge branch 'master' into vite-configts-fix

b0f78d7

n1ru4l approved these changes Mar 7, 2023

View reviewed changes

Delete .eslintignore

f010b06

dimaMachina approved these changes Mar 7, 2023

View reviewed changes

dimaMachina removed the request for review from charlypoly March 7, 2023 11:16

dimaMachina merged commit a34cef3 into master Mar 7, 2023

dimaMachina deleted the vite-configts-fix branch March 7, 2023 11:22

theguild-bot mentioned this pull request Mar 7, 2023

Upcoming Release Changes #9054

Merged

beerose mentioned this pull request Mar 9, 2023

show that type:module will break codegen #8988

Closed

MrTact mentioned this pull request Mar 11, 2023

Documentation: update for #9086? #9154

Closed

This was referenced Mar 15, 2023

codegen.ts and client-preset misses fragment when tsconfig.json contains allowJs: true #8986

Closed

Support for .mts files #8694

Closed

Support for .mts and .cts config files? #8303

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Load codegen.ts in ESM projects #9086

Load codegen.ts in ESM projects #9086

beerose commented Mar 1, 2023 •

edited

Loading

changeset-bot bot commented Mar 1, 2023 •

edited

Loading

github-actions bot commented Mar 1, 2023 •

edited

Loading

github-actions bot commented Mar 1, 2023 •

edited

Loading

enisdenjo left a comment

beerose commented Mar 2, 2023

enisdenjo commented Mar 2, 2023

enisdenjo left a comment

dimaMachina Mar 6, 2023

dimaMachina Mar 6, 2023

beerose Mar 6, 2023

dimaMachina Mar 6, 2023

dimaMachina Mar 6, 2023

beerose Mar 6, 2023

saihaj Mar 6, 2023 •

edited

Loading

dimaMachina Mar 6, 2023

beerose Mar 6, 2023 •

edited

Loading

beerose Mar 6, 2023

dimaMachina commented Mar 6, 2023

enisdenjo commented Mar 6, 2023 •

edited

Loading

beerose commented Mar 6, 2023

saihaj commented Mar 6, 2023

Load codegen.ts in ESM projects #9086

Load codegen.ts in ESM projects #9086

Conversation

beerose commented Mar 1, 2023 • edited Loading

Description

Type of change

How Has This Been Tested?

Checklist:

changeset-bot bot commented Mar 1, 2023 • edited Loading

🦋 Changeset detected

github-actions bot commented Mar 1, 2023 • edited Loading

🚀 Snapshot Release (alpha)

github-actions bot commented Mar 1, 2023 • edited Loading

💻 Website Preview

enisdenjo left a comment

Choose a reason for hiding this comment

beerose commented Mar 2, 2023

enisdenjo commented Mar 2, 2023

enisdenjo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saihaj Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beerose Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dimaMachina commented Mar 6, 2023

enisdenjo commented Mar 6, 2023 • edited Loading

beerose commented Mar 6, 2023

saihaj commented Mar 6, 2023

beerose commented Mar 1, 2023 •

edited

Loading

changeset-bot bot commented Mar 1, 2023 •

edited

Loading

github-actions bot commented Mar 1, 2023 •

edited

Loading

🚀 Snapshot Release (`alpha`)

github-actions bot commented Mar 1, 2023 •

edited

Loading

saihaj Mar 6, 2023 •

edited

Loading

beerose Mar 6, 2023 •

edited

Loading

enisdenjo commented Mar 6, 2023 •

edited

Loading