Allow for tree-shaking the Kind enum

[JoviDeCroock]

Resolves #4253

This enables tree-shaking Kind - I opted to only do this with Kind as the other enums are either tiny or included by default if you are using parseSchema.

[github-actions]

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

[jasonkuhrt]

I closed my duplicate here #4270.

[jasonkuhrt]

A name of kinds_ would keep it co-located with the other module that imports from this fwiw. Not sure what's consistent for this repo.

[jasonkuhrt]

Another approach that works I think is:

import * as kinds_ from './kinds_.js'; // eslint-disable-line

 export * as Kind from './kinds_.js';

 /**
  * The set of allowed kind values for AST nodes.
  */
 export type Kind = (typeof kinds_)[keyof typeof kinds_];

[JoviDeCroock]

@jasonkuhrt looks like not all TS versions support this

[jasonkuhrt]

@JoviDeCroock nice catch, yeah https://www.typescriptlang.org/dev/bug-workbench/?ts=4.8.4#code/FAj0oAgAQMwSwDYFMB2BDAtkgXBFB7AEyQH0MiBXZAZ1AHMAnNABwAsBHBUAazhUJ59CAOgAu1YEgAezfA1EQAxvhTUFAMQCSAUQAyAEQgBeCAHJ4SBIVOSZchctUKAQsbMAjUxHAQkoxcK2svJKKmoQAMJupopePn4BQfahThCGJqbW3pAJgUkhogCezEgQWnrpEEUl+DBlOgb5CtWlriYttRDOTVXFpVHtfZ0RPS1pbh11+iBgkLCIqJg4eESk5IRUSLSMLBxcvPygJCRiEnAYwc19EABUEGjUEADSQiQQMAz4GGbCgvzCACtqDZRtcXvwJkM6uDCCQANrcJCFTqTZ6vCAAMggagYfDoAF0QNJLrd7o8Ye9Pt9TL8DiIgSDZtB4Mh0FhcARiGRKDR6Ew2JxQEJpKcencPl8fn8BMdAcCZsAfPNWUtcCxmKLziSAN5oiEAX0pktMOwFCBBjnCMHw+DcAAo6bgYQBKYwAPgg2uAEB9EDgdQdQmMRgyFispldDD8FAYKGyEEItuoXz8rDxvogwH1IBhwnKBnjACIw4RC0yoOIALTEpCKUTVhifBjAa34O2mdxoayunwwNCIECt9sliPx-DcFs29ueHuQceTtsxUc+edDzLLufcIA

Can we drop support for 4.8? Does GraphQL to date support even older versions of TypeScript?

[Urigo]

maybe v17 is a good opportunity to drop support of old Typescript and Node versions?

[JoviDeCroock]

I think it's fair to cut https://github.com/graphql/graphql-js/blob/main/integrationTests/ts/package.json#L11-L22 down to i.e. 4.9 - recent but that might introduce some issues during migration - @yaacovCR has also expressed their concerns regarding ghosted exports which this also does so we might need to settle that first #4254 (comment)

[jasonkuhrt]

While writing this message #4254 (comment) I realized that we have an opportunity at the type level to improve the enum DX further.

Sometimes it is useful to be able to access the enum at the type level just like it would be at the term level e.g. Kind.FIELD // type level!.

As posted in that referenced message, here is a link demonstrating how to achieve that: https://www.typescriptlang.org/dev/bug-workbench/?#code/PTAEAEDMEsBsFMB2BDAtvAXKA1tRATAfQDoAXAZwCh4APABwHsAnU0AYwcXNeVAF5QARmr1mrUgE868ULwHCRjFu07dQAI36gATIrGhJ0jVt2UQEGAhTosuAmSq0lrAFSzyoANJ58oSEwZUUAByYmA7ImIAK3JgymhUZwMpGQBvWQAaYwBfPwCg0PCfEhi4vWVDGW8CfkpQAB9ZOsb1SjbzKDgkNEwDAHcGB3jE-XTq31z-QJCwiOjYto4uVnxocjpkUjYACy0ACgiMcYBKfgA+HB9F1R4tceJkSkrZO58H6+XjAXvW581vt7qIA

The difference is that:

1. We define each enum member type next to its term const variable.
2. When we define the Kind type we do so by importing the types from the module defining the members.

AFAICT this suggestion has no downside. Meanwhile its upside is increased symmetry between the term and type level and easier type level logic when type level access to specific enums are desired.

[jasonkuhrt]

Suggested change

	| typeof NAME
	| typeof DOCUMENT
	| typeof OPERATION_DEFINITION
	| typeof VARIABLE_DEFINITION
	| typeof SELECTION_SET
	| typeof FIELD
	| typeof ARGUMENT
	| typeof FRAGMENT_ARGUMENT
	| typeof FRAGMENT_SPREAD
	| typeof INLINE_FRAGMENT
	| typeof FRAGMENT_DEFINITION
	| typeof VARIABLE
	| typeof INT
	| typeof FLOAT
	| typeof STRING
	| typeof BOOLEAN
	| typeof NULL
	| typeof ENUM
	| typeof LIST
	| typeof OBJECT
	| typeof OBJECT_FIELD
	| typeof DIRECTIVE
	| typeof NAMED_TYPE
	| typeof LIST_TYPE
	| typeof NON_NULL_TYPE
	| typeof SCHEMA_DEFINITION
	| typeof OPERATION_TYPE_DEFINITION
	| typeof SCALAR_TYPE_DEFINITION
	| typeof OBJECT_TYPE_DEFINITION
	| typeof FIELD_DEFINITION
	| typeof INPUT_VALUE_DEFINITION
	| typeof INTERFACE_TYPE_DEFINITION
	| typeof UNION_TYPE_DEFINITION
	| typeof ENUM_TYPE_DEFINITION
	| typeof ENUM_VALUE_DEFINITION
	| typeof INPUT_OBJECT_TYPE_DEFINITION
	| typeof DIRECTIVE_DEFINITION
	| typeof SCHEMA_EXTENSION
	| typeof SCALAR_TYPE_EXTENSION
	| typeof OBJECT_TYPE_EXTENSION
	| typeof INTERFACE_TYPE_EXTENSION
	| typeof UNION_TYPE_EXTENSION
	| typeof ENUM_TYPE_EXTENSION
	| typeof INPUT_OBJECT_TYPE_EXTENSION;
	| NAME
	| DOCUMENT
	| ...

[jasonkuhrt]

Suggested change

	export const DOCUMENT = 'Document';
	export const OPERATION_DEFINITION = 'OperationDefinition';
	export const VARIABLE_DEFINITION = 'VariableDefinition';
	export const DOCUMENT = 'Document';
	export type DOCUMENT = typeof DOCUMENT
	export const OPERATION_DEFINITION = 'OperationDefinition';
	export type OPERATION_DEFINITION = typeof OPERATION_DEFINITION
	export const VARIABLE_DEFINITION = 'VariableDefinition';
	// ...

[JoviDeCroock]

The reason I did not do these is mainly that it would cause a huge pain with our eslint rule no-redeclare

[jasonkuhrt]

Would aslant complain about that? If so though, could we just ignore with an ignore directive at the top of the file?

[netlify]

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

Name	Link
🔨 Latest commit	c225a72
🔍 Latest deploy log	https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/672272e7565eef0008b7387a
😎 Deploy Preview	https://deploy-preview-4268--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.

[JoviDeCroock]

Going to close this as I won't have time to fine tune this. If someone wants to take this over feel free to, I might get back to it in the future.

[jasonkuhrt]

@JoviDeCroock Sure thing I'll take a swing, thanks for clearing the way up to this point!

[jasonkuhrt]

@JoviDeCroock @Urigo I've got something ready for review here #4270.