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(imports): Add emitLegacyCommonJSImports setting #121

Conversation

JustusFluegel
Copy link
Contributor

Closes #65

Sorry that this took so long, had a lot popping up and then kinda forgot I created this issue.

@changeset-bot
Copy link

changeset-bot bot commented Apr 10, 2023

🦋 Changeset detected

Latest commit: 2940182

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

This PR includes changesets to release 1 package
Name Type
@eddeee888/gcg-typescript-resolver-files 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

@eddeee888
Copy link
Owner

Hi @JustusFluegel , thank you for creating the PR! It's looking good. I left a few minor comments.

Here's a few extra things to do to make it ready to be merged:

  • Update branch with latest master
  • Generate an e2e test:
    $ nx workspace-generator typescript-resolver-files-add-e2e-test esm-import
  • Update the generated codegen.ts with the new config option:
    defineConfig({ emitLegacyCommonJSImports: false })
  • Generate file for e2e test:
    $ nx graphql-codegen typescript-resolver-files-e2e -c test-esm-import --verbose
  • Check to make sure all generated files are correct, then commit the generated files
  • Once done, run yarn changeset and follow the prompt. This can be a patch

@JustusFluegel
Copy link
Contributor Author

I think that looks good

╭────┬─────────────────────────────────────────────────╮
│  # │                    capture2                     │
├────┼─────────────────────────────────────────────────┤
│  0 │ ./../../types.generated.js                      │
│  1 │ ./../../types.generated.js                      │
│  2 │ graphql                                         │
│  3 │ ./types.generated.js                            │
│  4 │ ./topic/resolvers/Mutation/topicCreate.js       │
│  5 │ ./topic/resolvers/Mutation/topicEdit.js         │
│  6 │ ./base/resolvers/PaginationResult.js            │
│  7 │ ./base/resolvers/PayloadError.js                │
│  8 │ ./user/resolvers/Profile.js                     │
│  9 │ ./user/resolvers/Query/me.js                    │
│ 10 │ ./topic/resolvers/Query/topicById.js            │
│ 11 │ ./topic/resolvers/Query/topicsCreatedByUser.js  │
│ 12 │ ./user/resolvers/Query/userByAccountName.js     │
│ 13 │ ./base/resolvers/SomeRandomScalar.js            │
│ 14 │ ./user/resolvers/Subscription/profileChanges.js │
│ 15 │ ./topic/resolvers/Topic.js                      │
│ 16 │ ./topic/resolvers/TopicByIdPayload.js           │
│ 17 │ ./topic/resolvers/TopicByIdResult.js            │
│ 18 │ ./topic/resolvers/TopicCreatePayload.js         │
│ 19 │ ./topic/resolvers/TopicCreateResult.js          │
│ 20 │ ./topic/resolvers/TopicEditPayload.js           │
│ 21 │ ./topic/resolvers/TopicEditResult.js            │
│ 22 │ ./topic/resolvers/TopicsCreatedByUserPayload.js │
│ 23 │ ./topic/resolvers/TopicsCreatedByUserResult.js  │
│ 24 │ ./user/resolvers/User.js                        │
│ 25 │ ./user/resolvers/UserPayload.js                 │
│ 26 │ ./user/resolvers/UserResult.js                  │
│ 27 │ graphql-scalars                                 │
│ 28 │ ./../../../types.generated.js                   │
│ 29 │ ./../../../types.generated.js                   │
│ 30 │ ./../../../types.generated.js                   │
│ 31 │ ./../../../types.generated.js                   │
│ 32 │ ./../../types.generated.js                      │
│ 33 │ ./../../types.generated.js                      │
│ 34 │ ./../../types.generated.js                      │
│ 35 │ ./../../types.generated.js                      │
│ 36 │ ./../../types.generated.js                      │
│ 37 │ ./../../types.generated.js                      │
│ 38 │ ./../../types.generated.js                      │
│ 39 │ ./../../types.generated.js                      │
│ 40 │ ./../../types.generated.js                      │
│ 41 │ graphql                                         │
│ 42 │ graphql                                         │
│ 43 │ ./topic/topic.mappers                           │
│ 44 │ ./../../types.generated.js                      │
│ 45 │ ./../../../types.generated.js                   │
│ 46 │ ./../../../types.generated.js                   │
│ 47 │ ./../../../types.generated.js                   │
│ 48 │ ./../../types.generated.js                      │
│ 49 │ ./../../types.generated.js                      │
│ 50 │ ./../../types.generated.js                      │
│ 51 │ graphql-tag                                     │
├────┼─────────────────────────────────────────────────┤
│  # │                    capture2                     │
╰────┴─────────────────────────────────────────────────╯

@JustusFluegel
Copy link
Contributor Author

JustusFluegel commented Apr 15, 2023

oops ./topic/topic.mappers isn't correct.

Searched for half an hour but couldn't find out where that is coming from. Any ideas?

@eddeee888
Copy link
Owner

oops ./topic/topic.mappers isn't correct.

Searched for half an hour but couldn't find out where that is coming from. Any ideas?

Ah I see, mappers use the typescript-resolvers's mapper which can be set here

@JustusFluegel
Copy link
Contributor Author

I think your link is incorrect, it just redirects me to my last message

@eddeee888
Copy link
Owner

eddeee888 commented Apr 16, 2023

Oops, sorry 😅 . The correct link is here.

Signed-off-by: Justus Fluegel <justusfluegel@gmail.com>
Signed-off-by: Justus Fluegel <justusfluegel@gmail.com>
Signed-off-by: Justus Fluegel <justusfluegel@gmail.com>
@JustusFluegel JustusFluegel force-pushed the add-support-config-emitLegacyCommonJSImports branch from 9b46295 to 2618a57 Compare May 13, 2023 01:43
@JustusFluegel
Copy link
Contributor Author

sorry, should be done, should be ready for review if you have the time @eddeee888

@eddeee888
Copy link
Owner

eddeee888 commented May 13, 2023

Thanks @JustusFluegel! The implementation and e2e tests looks good!

There are some unit tests are failing here. Do you mind having a look?

I think we just need to add the flag in 🙂

You can run unit test by running this:

yarn nx test typescript-resolver-files

Signed-off-by: Justus Fluegel <justusfluegel@gmail.com>
@JustusFluegel
Copy link
Contributor Author

JustusFluegel commented May 16, 2023

Hopefully this should be it. If that was it then thanks for your patience :) @eddeee888

@eddeee888
Copy link
Owner

Great stuff! Thank you @JustusFluegel !

@eddeee888 eddeee888 merged commit af10b65 into eddeee888:master May 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Config option for using esm style .js imports
2 participants