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

Collection of libraries and how they import from graphql #4074

Open
phryneas opened this issue Apr 30, 2024 · 20 comments
Open

Collection of libraries and how they import from graphql #4074

phryneas opened this issue Apr 30, 2024 · 20 comments

Comments

@phryneas
Copy link

phryneas commented Apr 30, 2024

In the April meeting of the graphql-js-wg, there was discussion about bundling changes in the graphql package (#4062).
As these changes would also add an exports field, @benjie suggested that we might want to have a restrictive set of exports entry points (only graphql, maybe one or two more?), to discourage from deep-importing from implementation details we do not consider the public interface of the library and which would result in uninteded breaking changes upon e.g. code reorganization.

In this issue, we want to collect libraries and how they are importing from graphql, to ensure that this is a feasible idea so we can discuss this further in the next graphql-js-wg meeting.

If you are generally opposed to a change like this, this issue (or the next WG meeting) are a good place to write your concerns down :)

Please copy-paste this template into a comment and fill it out:

## `<library_name>`

### Where do you import from in your library?

* `graphql`

### (If you use deep imports) why do you use deep imports?

### Would it be okay for you only import from `graphql`, too?

* [ ] yes
* [ ] no
@phryneas
Copy link
Author

@apollo/client

Where do you import from in your library?

  • graphql

(If you use deep imports) why do you use deep imports?

Would it be okay for you only import from graphql, too?

  • yes
  • no

@phryneas
Copy link
Author

graphql-tag

Where do you import from in your library?

  • graphql
  • graphql/language/ast

(If you use deep imports) why do you use deep imports?

Maybe for historical reasons, but everything we import is also exported from graphql itself.

Would it be okay for you only import from graphql, too?

  • yes
  • no

@benjie
Copy link
Member

benjie commented May 1, 2024

grafast

Where do you import from in your library?

  • graphql
  • graphql/jsutils/PromiseOrValue
  • graphql/execution/execute
  • graphql/type/schema

(If you use deep imports) why do you use deep imports?

  • graphql/jsutils/PromiseOrValue
    • the type isn't exported directly, and redefining it in every file is annoying. Also, auto-import pulls it in
  • graphql/execution/execute (specifically the buildExecutionContext function)
    • In order to emulate GraphQLResolveInfo as used in GraphQL resolvers, Grafast needs to get the same execution context. It also needs to coerce the variables and do all the other things that buildExecutionContext does (and isn't where Grafast adds value); so it makes sense to use the underlying function.
  • graphql/type/schema
    • specifically the type GraphQLSchemaNormalizedConfig which is marked as internal but used as the return type of schema.toConfig(). Could be worked around via ReturnType<GraphQLSchema['toConfig']> or similar.

Would it be okay for you only import from graphql, too?

  • yes
  • no

@trevor-scheer
Copy link
Contributor

@apollo/server

Where do you import from in your library?

  • graphql

(If you use deep imports) why do you use deep imports?

Would it be okay for you only import from graphql, too?

  • yes
  • no

@kamilkisiela
Copy link

graphql-hive

Where do you import from in your library?

  • graphql

(If you use deep imports) why do you use deep imports?

  • import { validateSDL } from 'graphql/validation/validate' - as I only need to validate SDL, not build GraphQLSchema

Would it be okay for you only import from graphql, too?

  • yes
  • no

@n1ru4l
Copy link
Contributor

n1ru4l commented May 6, 2024

graphql-yoga

Where do you import from in your library?

  • graphql
  • graphql/jsutils/Maybe (only in tests for convenience)
  • graphql/validation/validate (only for test helpers)
  • graphql/validation/ValidationContext (for tests)

(If you use deep imports) why do you use deep imports?

See comments behind the import - we only use them in tests for testing specific features of the GraphQL server

Would it be okay for you only import from graphql, too?

  • yes (if the things are exported from the main graphql entry, as it would be annoying to double maintain the declarations ourself
  • no

@n1ru4l
Copy link
Contributor

n1ru4l commented May 6, 2024

@envelop/fragment-arguments

Where do you import from in your library?

  • graphql
  • graphql/language/lexer.js
  • graphql/language/parser.js

(If you use deep imports) why do you use deep imports?

Types for Parser and Lexer for extending and adding new functionality

Would it be okay for you only import from graphql, too?

  • yes, (if the types are explorer from the main graphql entry)
  • no

@enisdenjo
Copy link
Member

graphql-http

Where do you import from in your library?

  • graphql

(If you use deep imports) why do you use deep imports?

Would it be okay for you only import from graphql, too?

  • yes
  • no

@enisdenjo
Copy link
Member

graphql-sse

Where do you import from in your library?

  • graphql

(If you use deep imports) why do you use deep imports?

Would it be okay for you only import from graphql, too?

  • yes
  • no

@enisdenjo
Copy link
Member

graphql-ws

Where do you import from in your library?

  • graphql

(If you use deep imports) why do you use deep imports?

Would it be okay for you only import from graphql, too?

  • yes
  • no

@saihaj
Copy link
Member

saihaj commented May 6, 2024

graphql-codegen

Where do you import from in your library?

  • graphql

(If you use deep imports) why do you use deep imports?

Would it be okay for you only import from graphql, too?

  • yes
  • no

@dariuszkuc
Copy link

@apollo/composition

Where do you import from in your library?

  • graphql

(If you use deep imports) why do you use deep imports?

N/A

Would it be okay for you only import from graphql, too?

  • yes
  • no

@dariuszkuc
Copy link

dariuszkuc commented May 6, 2024

@apollo/federation-internals

Where do you import from in your library?

  • graphql
  • graphql/jsutils/Maybe
  • graphql/validation/validate
  • graphql/validation/ValidationContext
  • graphql/validation/specifiedRules

(If you use deep imports) why do you use deep imports?

Used for SDL validations.

Would it be okay for you only import from graphql, too?

  • yes (assuming those validations are re-exported from root graphql entry)
  • no

@dariuszkuc
Copy link

@apollo/gateway

Where do you import from in your library?

  • graphql
  • graphql/execution/values

(If you use deep imports) why do you use deep imports?

Use getVariableValues while parsing the request.

Would it be okay for you only import from graphql, too?

  • yes
  • no

@dariuszkuc
Copy link

@apollo/query-planner

Where do you import from in your library?

  • graphql

(If you use deep imports) why do you use deep imports?

N/A

Would it be okay for you only import from graphql, too?

  • yes
  • no

@dariuszkuc
Copy link

@apollo/subgraph

Where do you import from in your library?

  • graphql
  • graphql/validation
  • graphql/validation/validate
  • graphql/validation/ValidationContext
  • graphql/validation/specifiedRules
  • graphql/jsutils/PromiseOrValue

(If you use deep imports) why do you use deep imports?

Used for SDL validations.

Would it be okay for you only import from graphql, too?

  • yes (assuming those validations are re-exported from root graphql entry)
  • no

@jasonkuhrt
Copy link

Thanks @phryneas I just pushed a fix that removed deep imports wherever I can. The following is up to date with that.


graphql-request

Where do you import from in your library?

import { ... } from 'graphql'
import { type ObjMap } from 'graphql/jsutils/ObjMap.js'

(If you use deep imports) why do you use deep imports?

To access ObjMap which is the type of some data structures exposed by graphql library.

Would it be okay for you only import from graphql, too?

  • yes [1]
  • no

[1] Except for ObjMap type which is currently not exported from graphql. Worst case I could copy the type into my package.

jasonkuhrt added a commit to graffle-js/graffle that referenced this issue May 9, 2024
This makes the library better prepared for future versions of `graphql`
package.

See: graphql/graphql-js#4074
@AnthonyMDev
Copy link

AnthonyMDev commented May 10, 2024

apollo-ios

Where do you import from in your library?

  • graphql
  • graphql/validation/validate
  • graphql/language/ast

(If you use deep imports) why do you use deep imports?

To access validateSDL (validation) and isNode (ast)

Would it be okay for you only import from graphql, too?

  • yes
  • no

@dylanaubrey
Copy link

graphql-box

Where do you import from in your library?

  • graphql
  • graphql/jsutils/PromiseOrValue

(If you use deep imports) why do you use deep imports?

Would it be okay for you only import from graphql, too?

  • yes
  • no

@benjie
Copy link
Member

benjie commented Jun 18, 2024

A new library I'm working on needs isNode to be exported (currently imported from graphql/language/ast).

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

No branches or pull requests