Backport 16.x.x into main #4165

JoviDeCroock · 2024-08-15T07:12:09Z

We could backport #3730 to v17 as it's reverted in 16.x.x CC @n1ru4l

This gets us to a linear history - in the future as we merge things into 16.x.x let's ensure we leverage merge commits

@IvanGoncharov

…tional arguments (#3645) BACKPORT OF #3634 Deprecates the positional arguments to createSourceEventStream, to be removed in the next major version, in favor of named arguments. Motivation: 1. aligns createSourceEventStream with the other exported entrypoints graphql, execute, and subscribe 2. allows simplification of mapSourceToResponse suggested by @IvanGoncharov

Co-authored-by: Yaacov Rydzinski <yaacovCR@gmail.com>

#3707) Co-authored-by: Ivan Goncharov <ivan.goncharov.ua@gmail.com>

…3838)

Closes #3758

Fixes: #3919 #3920 #3921

fixes from #3958 into

@n1ru4l

#4022) As surfaced in [Discord](https://discord.com/channels/625400653321076807/862957336082645006/1206980831915282532) this currently is a breaking change in the 16.x.x release line which is preventing folks from upgrading towards a security fix. This PR should result in a patch release on the 16 release line. This change was originally introduced to support CFW and browser environments which should still be supported with the `typeof` check CC @n1ru4l This also adds a check whether `.env` is present as in the DOM using `id="process"` defines that as a global which we don't want to access on accident. as shown in #4017 Bundles also target `process.env.NODE_ENV` specifically which fails when it replaces `globalThis.process.env.NODE_ENV` as this becomes `globalThis."production"` which is invalid syntax. Fixes #3978 Fixes #3918 Fixes #3928 Fixes #3758 Fixes #3934 This purposefully does not account for #3925 as we can't address this without breaking CF/plain browsers so the small byte-size increase will be expected for bundled browser environments. As a middle ground we did optimise the performance here. We can revisit this for v17. Most bundlers will be able to tree-shake this with a little help, in #4075 (comment) you can find a conclusion with a repo where we discuss a few. - Next.JS by default replaces [`process.env.NODE_ENV`](https://github.com/vercel/next.js/blob/b0ab0fe85fe8c93792051b058e060724ff373cc2/packages/next/webpack.config.js#L182) you can add `typeof process` linearly - Vite allows you to specify [`config.define`](https://vitejs.dev/config/shared-options.html#define) - ESBuild by default will replace `process.env.NODE_ENV` but does not support replacing `typeof process` - Rollup has a plugin for this https://www.npmjs.com/package/@rollup/plugin-replace Supersedes #4021 Supersedes #4019 Supersedes #3927 > This now also adds a documentation page on how to remove all of these

Co-authored-by: enisdenjo <denis@denelop.com> Co-authored-by: Denis Badurina <badurinadenis@gmail.com>

…4124) Co-authored-by: Erik Kessler <erik.kessler1@gmail.com> Co-authored-by: Benedikt Franke <benedikt.franke@mll.com> Co-authored-by: Michael Hayes <michael@hayes.io> Co-authored-by: Mike Ciesielka <maciesielka@comcast.net>

… as a thunk (#4122)

Co-authored-by: Saihajpreet Singh <saihajpreet.singh@gmail.com>

netlify · 2024-08-15T07:12:26Z

✅ Deploy Preview for compassionate-pike-271cb3 ready!

Name	Link
🔨 Latest commit	`58aca55`
🔍 Latest deploy log	https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/66bef346d6a68d000831f102
😎 Deploy Preview	https://deploy-preview-4165--compassionate-pike-271cb3.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

github-actions · 2024-08-15T07:12:35Z

Hi @JoviDeCroock, I'm @github-actions bot happy to help you with this PR 👋

Supported commands

Please post this commands in separate comments and only one per comment:

@github-actions run-benchmark - Run benchmark comparing base and merge commits for this PR
@github-actions publish-pr-on-npm - Build package from this PR and publish it on NPM

benjie · 2024-08-15T08:54:38Z

It would be wise to look over https://github.com/graphql/graphql-js/blob/16.x.x/resources/gen-changelog.js and see if it supports merge commits or if support can be added easily. If not, it may make sense to move to something like changesets before this merge? Whatever we do, it should be at least as good as what we have already.

.github/workflows/pull_request.yml

.github/workflows/push.yml

JoviDeCroock · 2024-08-15T15:36:19Z

@benjie you are right

Error: Commit 1f8ba95c662118452bd969c6b26ba4e9050c55da has no associated PR: 16.5.0

yaacovCR · 2024-08-15T20:51:47Z

It looks like the changes center around the following three areas:

Readme changes around graphql conf
Enum changes
instanceof changes and documentation for v16

Maybe we can:

submit three separate pull requests for these three issues into v17
run the change log script / release
do the merge commit

in terms of going forward:

I think a solution is to just release v17 and then go back to having main be stable again ie continue to represent v17 for time being and then any v18 be a separate feature branch

Are we ready to release v17? I think so! All the changes around incremental delivery are only around refactoring with the actual format stable for quite some time.

yaacovCR · 2024-08-15T21:20:09Z

Two additional points:

This approach would mean that the merge commit itself has no substantial changes which I think would make the history more readily understandable. There would be a single merge commit without any actual changes only for the purpose of creating a non conflicting diff between v16.x.x branch and v17 / main — I am not 100% certain of the importance of creating this non-conflicting diff, but…

In terms of readiness to release v17 — I mean, incremental delivery is ready to release under the experimental tags not that it is ready for actual release.

JoviDeCroock · 2024-08-16T06:30:26Z

Are we ready to release v17? I think so! All the changes around incremental delivery are only around refactoring with the actual format stable for quite some time.

I think that is a working group discussion and shouldn't be done lightly, I know there are more things that needed attention from previous WG meetings. In particular the dual package hazard as well as removing instanceof.

This approach would mean that the merge commit itself has no substantial changes which I think would make the history more readily understandable. There would be a single merge commit without any actual changes only for the purpose of creating a non conflicting diff between v16.x.x branch and v17 / main — I am not 100% certain of the importance of creating this non-conflicting diff, but…

It makes the git blame accurate with backrefs to relevant PR's and explanations.

If not, it may make sense to move to something like changesets before this merge? Whatever we do, it should be at least as good as what we have already.

I think either we move to changesets or for this one release we do a manual changelog, I don't mind the latter and moving to changesets after the fact. My main concern is leaving this branch open for too long CC @benjie

Alternatively we add --first-parent to the revList retrieval.

yaacovCR · 2024-08-16T07:18:40Z

It makes the git blame accurate with backrefs to relevant PR's and explanations.

We could have that by frontporting the enum changes, instanceof changes (and the banner) to main without the merge commit? Meaning git blame would identify those lines. It was my understanding that merge commits make git blame more difficult, although I am less familiar with that workflow.

I didn’t mean to be cavalier btw about releasing v17 — apologies!. Just excited in that I believe the incremental delivery experiment should no longer be considered to be blocking.

JoviDeCroock · 2024-08-16T07:24:07Z

We could have that by frontporting the enum changes, instanceof changes (and the banner) to main without the merge commit? Meaning git blame would identify those lines. It was my understanding that merge commits make git blame more difficult, although I am less familiar with that workflow.

I understand but it would also take away the contributions from the folks who committed time to improving v16, from that point of view I personally prefer this approach over finding a way around our changelog currently lacking.

I didn’t mean to be cavalier btw about releasing v17 — apologies!. Just excited in that I believe the incremental delivery experiment should no longer be considered to be blocking.

No need to apologize at all, if I came across in any blame way I do apologize. I am very excited about incremental delivery but a new major is always a big effort for our consumers and I personally always fear leaving people behind (if we look at v14/v15, there's still a lot of folks left behind). My reasoning is more in that area, when we cut a major I would love for everyone to chime in and agree that we have everything we want in this major.

yaacovCR · 2024-08-19T04:39:49Z

@JoviDeCroock If you have time to take a look at #4171 and the discussion I included there, I am trying to show that I think that the very valid concerns around both linear history and author credit can both be addressed whilst still preserving the linear history on main.

benjie · 2024-09-17T11:37:44Z

Should this be closed now?

IvanGoncharov and others added 29 commits May 9, 2022 19:30

Expose GraphQLErrorOptions type (#3554) (#3565)

1112b4d

16.5.0

1f8ba95

Workaround for codesandbox having bug with TS enums (#3686)

af8221a

Parser: allow 'options' to explicitly accept undefined (#3701)

6c6508b

parser: limit maximum number of tokens (#3702)

f0a0a4d

Co-authored-by: Yaacov Rydzinski <yaacovCR@gmail.com>

16.6.0

3a51eca

Fix crash in node when mixing sync/async resolvers (backport of #3706) (

4a82557

#3707) Co-authored-by: Ivan Goncharov <ivan.goncharov.ua@gmail.com>

Fix/invalid error propagation custom scalars (backport for 16.x.x) (#…

076972e

…3838)

check "globalThis.process" before accessing it (#3887)

84bb146

Closes #3758

16.7.0

1519fda

instanceOf: workaround bundler issue with process.env (#3923)

a08aaee

Fixes: #3919 #3920 #3921

16.7.1

bf6a9f0

Support fourfold nested lists (#3950)

bec1b49

16.8.0

e4f759d

OverlappingFieldsCanBeMergedRule: Fix performance degradation (#3967)

8f4c64e

fixes from #3958 into

16.8.1

8a95335

Fix publish scripts (#4104)

c82609e

16.8.2

08779a0

backport[v16]: Introduce "recommended" validation rules (#4119)

c985c27

Co-authored-by: enisdenjo <denis@denelop.com> Co-authored-by: Denis Badurina <badurinadenis@gmail.com>

feat: allow defining symbol error extensions (#3730)

29c1bff

Revert error extension symbol (#4123)

c35130e

backport[v16]: Enable passing values configuration to GraphQLEnumType…

6a1614c

… as a thunk (#4122)

16.9.0

556a01e

Upgrade codecov action and pass token (#4138)

04cba88

Fix codecov workflow (#4139)

e1ceb8f

Add GraphQLConf 2024 banner (#4157)

9a91e33

Co-authored-by: Saihajpreet Singh <saihajpreet.singh@gmail.com>

JoviDeCroock requested a review from a team as a code owner August 15, 2024 07:12

JoviDeCroock requested review from benjie and removed request for a team August 15, 2024 07:12

JoviDeCroock force-pushed the backport-16.x.x branch 2 times, most recently from b95bc51 to 7753133 Compare August 15, 2024 07:17

JoviDeCroock changed the title ~~Backport 16.x.x~~ Backport 16.x.x into main Aug 15, 2024

JoviDeCroock requested a review from a team August 15, 2024 07:28

yaacovCR reviewed Aug 15, 2024

View reviewed changes

.github/workflows/pull_request.yml Outdated Show resolved Hide resolved

.github/workflows/push.yml Outdated Show resolved Hide resolved

JoviDeCroock force-pushed the backport-16.x.x branch from 7753133 to f412f28 Compare August 16, 2024 06:23

JoviDeCroock force-pushed the backport-16.x.x branch from f412f28 to 6687fc0 Compare August 16, 2024 06:30

JoviDeCroock mentioned this pull request Aug 16, 2024

chore(deps): upgrade dependencies #4166

Merged

JoviDeCroock force-pushed the backport-16.x.x branch from 6687fc0 to 58aca55 Compare August 16, 2024 06:35

Merge remote-tracking branch 'origin/16.x.x' into backport-16.x.x

58aca55

yaacovCR mentioned this pull request Aug 19, 2024

chore: update main with outstanding fixes on v16.x.x branch #4171

Merged

yaacovCR mentioned this pull request Aug 19, 2024

Notice: default branch is now 16.x.x #4131

Open

JoviDeCroock closed this Sep 17, 2024

JoviDeCroock deleted the backport-16.x.x branch October 13, 2024 14:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Backport 16.x.x into main #4165

Backport 16.x.x into main #4165

JoviDeCroock commented Aug 15, 2024 •

edited

Loading

netlify bot commented Aug 15, 2024 •

edited

Loading

github-actions bot commented Aug 15, 2024

benjie commented Aug 15, 2024

JoviDeCroock commented Aug 15, 2024 •

edited

Loading

yaacovCR commented Aug 15, 2024

yaacovCR commented Aug 15, 2024

JoviDeCroock commented Aug 16, 2024 •

edited

Loading

yaacovCR commented Aug 16, 2024

JoviDeCroock commented Aug 16, 2024 •

edited

Loading

yaacovCR commented Aug 19, 2024

benjie commented Sep 17, 2024

Backport 16.x.x into main #4165

Backport 16.x.x into main #4165

Conversation

JoviDeCroock commented Aug 15, 2024 • edited Loading

netlify bot commented Aug 15, 2024 • edited Loading

✅ Deploy Preview for compassionate-pike-271cb3 ready!

github-actions bot commented Aug 15, 2024

benjie commented Aug 15, 2024

JoviDeCroock commented Aug 15, 2024 • edited Loading

yaacovCR commented Aug 15, 2024

yaacovCR commented Aug 15, 2024

JoviDeCroock commented Aug 16, 2024 • edited Loading

yaacovCR commented Aug 16, 2024

JoviDeCroock commented Aug 16, 2024 • edited Loading

yaacovCR commented Aug 19, 2024

benjie commented Sep 17, 2024

JoviDeCroock commented Aug 15, 2024 •

edited

Loading

netlify bot commented Aug 15, 2024 •

edited

Loading

JoviDeCroock commented Aug 15, 2024 •

edited

Loading

JoviDeCroock commented Aug 16, 2024 •

edited

Loading

JoviDeCroock commented Aug 16, 2024 •

edited

Loading