Skip to content

Latest commit

 

History

History
101 lines (99 loc) · 7.58 KB

2024-04.md

File metadata and controls

101 lines (99 loc) · 7.58 KB

GraphQL WG Notes - April 2024

Watch the replays: GraphQL.js Working Group Meetings on YouTube

  • Intros, agreement, etc.
  • Agenda item: ESM, dual package hazard
  • Lenz: recap how I think we’ve gotten here
  • There was a plan to make graphqljs ESM-only
  • (no one on the call claims knowledge of this history 🙂)
  • ESM + CJS modules are distinct from each other in each working version (16, 17)
  • Our colleague Alessia dove into testing a lot - couldn’t get vitest running w/ graphqljs
  • Vitest tries to import ESM everywhere, but some libs are CJS-based - if there’s a nested graphqljs dependency, that’s a conflict
  • Convo 1y ago with maintainer of xstate, redux-saga, etc.
  • Tl;dr it’s extremely complicated
  • Benjie: even w/ one version installed, can have two versions in memory
  • Also can cause multiple versions to be resolved/installed
  • I think the approach of node only able to import it as CJS makes sense - where I see problems (also ESM shim is a factor) - number of situations where people are bundling server-side nodejs, not 100% sure they’re bundling the entire thing, worried there may be a crossover
  • AWS Lambda, Serverless come to mind
  • Also Next.js
  • Not 100% sure that will work out
  • Lenz: deep imports are a factor. Hopefully it could work w/ wildcards - haven’t tested it out yet
  • Benjie: not sure the intent is to import from subpaths. Entire API is available via direct import
  • Some utilities exist that aren’t top-level but technically not part of public interface - we reserve right to break those
  • Using export maps to be explicit would be desirable. If there are utilities people need that are accessible via subpaths, either export the subpaths or export them top-level
  • Lenz: ofc we have that right in a major version to break it
  • If we try to use vitest and have old libs, you can sub in graphql v17 alpha - new part of ecosystem is broken, can’t use that
  • A bit afraid of doing that breaking change - could also roll up whole build into one file
  • Could probably make it easier to tree-shake with the right annotations in resulting bundle
  • Benjie: good point - anyone importing from subpaths should request that they’re top-level exports
  • V17 upgrade would be replacing graphql/something to just graphql directly
  • Lenz: my concern is libs, not necessarily apps
  • Benjie: would necessitate new published versions or patch-package
  • If we’re going to adopt exports it should be done in a way that is intentional
  • Good time to make private APIs explicitly private
  • Best time to do it is when we release the first version
  • It would definitely make it a lot easier to do all those changes
  • Lenz: Not opposed to it
  • Benjie: I’m a host of this meeting, not a maintainer, so just my opinions, not necessarily the opinion of the project
  • (Discussing these comments: graphql/graphql-js#4062 (comment) )
  • Any ESM-only projects that don’t honor “mjs”
  • Lenz: Maybe browser?
  • If someone could consume this from a CDN, they would have to point to the module entrypoint or the CDN would have to auto-transpile it
  • On the other hand, both would probably automatically happen
  • Benjie: something to research?
  • Lenz: To a certain degree, I’m positive toward that direction, hopeful, based on other projects
  • Benjie: what next? Have the feedback you need?
  • Lenz: I’ll do a rollup like you said - for a restart, to enter from index.js - some kind of rollup
  • If I were to go in that direction, I would have everything, question is should I?
  • Benjie: more of a user question, rather than something for this WG itself
  • Maybe ask in the main GraphQL WG?
  • Ex: The Guild might be using subpaths for some of their projects, like the parser, haven’t tried this
  • Lenz: a few I could think of from packages we maintain
  • Rationale behind directly importing would be to assume it’s easier to tree-shake for all bundlers that consume our lib
  • Not sure which consumers - make it as easy as possible - valid point
  • Benjie: Won’t be to the granularity of every file used
  • Could regex search all of GitHub
  • Jeff: is v17 the right place for this?
  • Lenz: new ecosystem now, dual-package hazard is a thing
  • We are creating exports field, would then become an official entrypoint
  • If we did that for each file, we will be closing the door on what Benjie suggested. Might be extremely confusing
  • Benjie: my understanding from Ivan - v17 is effectively a playground for incremental delivery
  • Were intending to get that out, but protocol has issues that are being worked out. Nearing the end, not ready to release
  • V16 is latest stable
  • Would branch off latest release of 16, add changes to that. TS/JS file changes are likely minimal, try to keep it that way
  • I could see something like this being released as a v17 alone, bump current v17 feature set to v18
  • For the time being there’s no maintainer, so we’ll do group decisions until we have a leader
  • Next steps: ESM file, give it a go, ecosystem question RE: subpath imports, which should be explicitly public vs. private
  • Non-bundler, non-node, what are they and how would they be affected?
  • Suggestion: write a brief, concise description of what you’re looking for. 1-2m agenda topic - no questions, just me telling you what I’m planning to do - send me feedback asynchronously
  • Many who don’t attend do look at the agenda
  • Also happy to represent
  • Other topics?
  • Lenz: lot of back-and-forth RE: five PRs, globalThis check
  • graphql/graphql-js#4022
  • Benjie: Looking for feedback from Laurent
  • Lenz: in browsers, module import of graphql will crash
  • Esbuild cannot replace typeof process - therefore can’t tree-shake, which is the arg against that
  • Vite does regex replacement. Esbuild looks at AST because typeof is not a node they would look at - esbuild maintainer has said they won’t change because it would be “wrong”
  • Benjie: what this says to me is an import from graphql and graphql/dev
  • Lenz: can split on import statement (...)
  • That would mean bundlers that don’t pick up on development/production, would see weird thing, but bundlers that depend on dev/prod would pick it up, more would adopt over time
  • Check for globalThis.process, …env, etc.
  • Ok to move back into GH Discussion?
  • Benjie: would be a good idea - just one thought - is the issue the fact with globalThis existing is causing further problems?
  • Lenz: it’s causing problems but only in bundlers that do wrong stuff w/o string replacements w/o checking start of var name
  • Benjie: Can we do const process = globalThis.process?
  • Lenz: It’s running in circles, bundlers wouldn’t follow that
  • Will look up pros/cons, discuss w/ people - feel bad putting in my opinion vs. others
  • Benjie: seems relatively urgent - don’t have the right ppl in meeting to make decision
  • Proposal - if this can wait 5-6wks - I will post that we will discuss one last time, get feedback in, we’re going to make a decision and follow through
  • Effectively, here’s what we want to do
  • Lenz: fine w/ me, waiting 1-2 mos is fine
  • Benjie: last stable release?
  • Jerel: 19 Sept 2023
  • Benjie: probably just revert to previous release - what you have here though looks fine
  • Lenz: this is the one I don’t agree on
  • Talking about binary logic w/o writing is pretty pointless
  • Benjie: take action of writing a summary of this problem?
  • Lenz: will do - write it up, next graphqljs wg, also next primary wg
  • Benjie: next one is next week