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

fix: align cjs/esm hybrid exports #815

Closed
wants to merge 1 commit into from

Conversation

igl
Copy link

@igl igl commented Sep 4, 2024

Hello,

i have been trying to get dagger.io running on deno and i have stumbled on an issue importing graphql-tag in deno.
See: denoland/deno#25311

graphql-tag bundles into an umd.js file that masquerades as an esm-module by having a "default" and __esModule property. But package.json declares another the ./main.js as it's primary module. This one exclude all of the umd bundle and simply exports the gql function.

graphql-tag hasn't seen an update published in 3 years and there is properly much more todo to renovate the build process.

This PR alleviates the problem with being a hybrid package a little bit without breaking legacy imports.

Thank you for reading!

@igl igl requested a review from a team as a code owner September 4, 2024 19:34
@apollo-cla
Copy link

@igl: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@beingminimal
Copy link

@igl Please solve this as soon as possible, so we can use dagger through deno seamlessly.
Thanks a lot for your efforts.

@DerZade
Copy link

DerZade commented Jan 7, 2025

I just stumbled on this trying to get graphql-tag working with Deno and this kinda makes my head hurt. You're fixing the CJS export (which is just a reexport of the the UMD ...WTF) to ensure Deno can load it as ESM?
I totally get that this is not the fault of this PR, but due to the dated build setup, but just WTF

Since the bundle actually includes a ESM version (lib/index.js) it would probably be cleaner to try to get Deno to just use that directly. This should work by adding an exports field to the package.json which includes an import attribute:

  "exports": {
    ".": {
      "types": "./lib/index.d.ts",
      "import": "./lib/index.js",
      "require": "./main.js"
    }
  },

Deno and Node (at least the newer versions) should prefer this over main:

If both "exports" and "main" are defined, the "exports" field takes precedence over "main" in supported versions of Node.js.

— the Node.js docs

@jerelmiller
Copy link
Member

Hey @igl 👋

We are planning to publish a new major version of graphql-tag with the release of Apollo Client 4.0 which will address some existing ESM issues. As such, I'm going to close this PR. Follow along with the 3.0 milestone if you'd like to be informed of progress. Thanks for the contribution regardless!

@jerelmiller jerelmiller closed this Jan 8, 2025
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.

5 participants