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

Richer import syntax #9

Merged
merged 3 commits into from
Jul 26, 2019
Merged

Richer import syntax #9

merged 3 commits into from
Jul 26, 2019

Conversation

dfreeman
Copy link
Owner

@dfreeman dfreeman commented Jul 25, 2019

This PR (built on top of the diff for #8) introduces support for graphql-import-style import directives, as outlined in #3. While there's still no official spec for modularizing GraphQL schemas and operations across files, the graphql-import package gets over half a million downloads a week, so seems like the likeliest thing we should attempt syntactic compatibility with for the time being.

A couple thoughts/questions:

  • Should we explicitly deprecate the old syntax? Like-for-like migration should be pretty straightforward: find/replace #import ➡️ # import * from in all .graphql files in a given codebase.
  • This doesn't support renaming identifiers as described in Fragment can't be imported more than once #1 (and graphql-import doesn't support that at all), but it at least opens the door to it if that's something folks decide is worth investing in down the line. I think I was misremembering the heart of that issue. The way this actually stands, if you want to import and use Foo, and it references Bar, you'd have to # import Foo, Bar from ... in order for that to work. That does solve Fragment can't be imported more than once #1, but is probably more painful in the general case—maybe that's just when you fall back to import * 🤔

@csantero
Copy link
Collaborator

@dfreeman Yeah, let's deprecate. Could you print a message that includes the name of the file that the legacy import occurs in, as well a blurb to the readme with migration instructions?

@csantero
Copy link
Collaborator

@dfreeman I just tried this PR in my own app and it works great, before and after migrating syntax with the global search-and-replace. import * is fine for my own use-case, since all my files have exactly one top-level query or fragment.

@dfreeman
Copy link
Owner Author

@csantero Glad to hear it! My main motivator for this update was wanting to be able to tell syntactically which fragments come from which imports for TypeScript reasons, but I imagine for many folks # import * will be just fine.

I added a deprecation message that links to a new blurb in the README about migration, so hopefully this should be good to go now 🙂

@csantero csantero merged commit 65617d1 into dfreeman:master Jul 26, 2019
@csantero
Copy link
Collaborator

This is great, thanks!

@csantero
Copy link
Collaborator

Released as 0.4.1

@dfreeman dfreeman deleted the richer-import-syntax branch July 27, 2019 05:15
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.

2 participants