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

refactor: treeshakable kind enum #4269

Closed
wants to merge 39 commits into from

Conversation

jasonkuhrt
Copy link

@jasonkuhrt jasonkuhrt commented Oct 29, 2024

closes #4253

IvanGoncharov and others added 30 commits May 9, 2022 19:30
…tional arguments (graphql#3645)

BACKPORT OF graphql#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
graphql#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 graphql#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 graphql#3978
Fixes graphql#3918
Fixes graphql#3928
Fixes graphql#3758
Fixes graphql#3934

This purposefully does not account for
graphql#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
graphql#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 graphql#4021
Supersedes graphql#4019
Supersedes graphql#3927

> This now also adds a documentation page on how to remove all of these
…raphql#4124)

Co-authored-by: Erik Kessler <[email protected]>
Co-authored-by: Benedikt Franke <[email protected]>
Co-authored-by: Michael Hayes <[email protected]>
Co-authored-by: Mike Ciesielka <[email protected]>
In response to some of our actions starting to fail deprecate all of the
actions that don't work anymore due to using Node 10/12

- [Looks like upload artefact is safe to
upgrade](https://github.com/actions/upload-artifact/blob/main/docs/MIGRATION.md)
we don't seem to overwrite files
- [Same for
download](https://github.com/actions/download-artifact/blob/main/docs/MIGRATION.md)
benjie and others added 9 commits September 18, 2024 17:47
…th nested fragments (graphql#4168)

In trying to prevent infinite loops, graphql#3442 introduced a bug that causes certain violations of the [Field Selection Merging](https://spec.graphql.org/draft/#sec-Field-Selection-Merging) validation to not be caught (released in `16.3.0`, and backported to `15.9.0`). This PR fixes this bug, while continuing to prevent infinite loops.
This converts the existing docusaurus website to nextra just like
https://github.com/graphql/graphql.github.io. This is a first step in
moving the documentation here and having a redirect from graphql.org to
graphql-js.org.

Not sure yet why codecov started failing 😅 when I run `testonly:cover`
locally it tells me we are 100% covered.

WDYT about isolating the dependencies for the website in that folder? As
seen in
graphql@9c7d615
this prevents the weird CI leaks that we're seeing

Resolves graphql#4200

---------

Co-authored-by: Yaacov Rydzinski <[email protected]>
Checked for broken styles and fixed them and also went over the links by
using linkinator. I did check a lot of tools but looks like there isn't
a good way at the moment to check for broken links that is maintained.

Resolves graphql#4242
@jasonkuhrt jasonkuhrt requested a review from a team as a code owner October 29, 2024 14:52
Copy link

CLA Not Signed

Copy link

Hi @jasonkuhrt, 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

@jasonkuhrt jasonkuhrt changed the base branch from 16.x.x to main October 29, 2024 14:53
@jasonkuhrt jasonkuhrt closed this Oct 29, 2024
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.

Enums not tree shaking well