Skip to content

Install graphql-js into devDependencies at the monorepo level.#1542

Merged
abernix merged 4 commits intomasterfrom
abernix/add-graphql-js-into-devDependencies
Sep 30, 2019
Merged

Install graphql-js into devDependencies at the monorepo level.#1542
abernix merged 4 commits intomasterfrom
abernix/add-graphql-js-into-devDependencies

Conversation

@abernix
Copy link
Copy Markdown
Member

@abernix abernix commented Sep 25, 2019

Currently, even though most of this repository currently specifies graphql as a peer-dependency, the only reason it's installed at the monorepo root, and thus available for use by all packages, is because it's a direct dependency of apollo and apollo-language-server.

This is causing it not to be upgraded to the most recent version by Renovate. In my own case, I needed the latest version for TypeScript typings for PossibleTypeExtensions which were not ever officially published in the DefinitelyTyped typings (likely due to the method being
experimental) but are now part of the graphql package itself thanks to the
work that @JacksonKearl did.

Currently, even though most of this repository currently specifies `graphql`
as a peer-dependency, the only reason it's installed at the monorepo root,
and thus available for use by all packages, is because it's a direct
dependency of `apollo` and `apollo-language-server`.

This is causing it not to be upgraded to the most recent version by
Renovate.  In my own case, I needed the latest version for TypeScript
typings for `PossibleTypeExtensions` which were not ever officially
published in the DefinitelyTyped typings (likely due to the method being
experimental) but are now part of the `graphql` package itself thanks to the
work that @JacksonKearl did.
@abernix
Copy link
Copy Markdown
Member Author

abernix commented Sep 25, 2019

Test failures seem legitimate! Though I'll have to circle back to this later, so I'll close this for now.

@abernix abernix closed this Sep 25, 2019
@abernix
Copy link
Copy Markdown
Member Author

abernix commented Sep 25, 2019

cc @trevor-scheer

@trevor-scheer
Copy link
Copy Markdown
Contributor

@abernix is this change still of interest? Rather, what's the reason for closing?

@abernix abernix reopened this Sep 27, 2019
@abernix
Copy link
Copy Markdown
Member Author

abernix commented Sep 27, 2019

@trevor-scheer I’d appreciate your input on it, I mainly didn’t want to clog up the PR queue with something I hadn’t really had time to look into at all.

I do have another branch which depends on this change for the typings. But I could also feel okay opening that branch with a ts-ignore.

Input appreciated though, if you have time!

@trevor-scheer trevor-scheer force-pushed the abernix/add-graphql-js-into-devDependencies branch from 251f339 to 186e381 Compare September 27, 2019 17:09
@trevor-scheer
Copy link
Copy Markdown
Contributor

@abernix this should be good to go now!

There was a TS error that resolved the build issues, here:
https://github.com/apollographql/apollo-tooling/pull/1542/files#diff-9743ccdba5e7089773d96956dd6e2e42R240

extensions is a Maybe<Record<...>> so I used undefined since this previously wasn't a thing.

I also removed the @types/graphql dev dependency since those types are now included (and I'm unsure what kind of behavior to expect with those existing side by side).

@abernix
Copy link
Copy Markdown
Member Author

abernix commented Sep 30, 2019

Removing the @types/graphql seems correct to me and using undefined here makes sense to me as well! Thanks so much for taking a look at this, @trevor-scheer!

@abernix abernix merged commit 0ddfc0a into master Sep 30, 2019
@abernix abernix deleted the abernix/add-graphql-js-into-devDependencies branch September 30, 2019 10:35
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