fix: make `@types/codemirror` a dependency of `@graphiql/react`

[SimenB]

Without this, I get type errors in my project

../../node_modules/@graphiql/react/types/editor/components/image-preview.d.ts:1:28 - error TS7016: Could not find a declaration file for module 'codemirror'. '/Users/simen/repos/monorepo/node_modules/codemirror/lib/codemirror.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/codemirror` if it exists or add a new declaration (.d.ts) file containing `declare module 'codemirror';`

1 import type { Token } from 'codemirror';
                             ~~~~~~~~~~~~

../../node_modules/@graphiql/react/types/editor/hooks.d.ts:2:42 - error TS7016: Could not find a declaration file for module 'codemirror'. '/Users/simen/repos/monorepo/node_modules/codemirror/lib/codemirror.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/codemirror` if it exists or add a new declaration (.d.ts) file containing `declare module 'codemirror';`

2 import type { EditorConfiguration } from 'codemirror';
                                           ~~~~~~~~~~~~

../../node_modules/@graphiql/react/types/editor/response-editor.d.ts:1:38 - error TS7016: Could not find a declaration file for module 'codemirror'. '/Users/simen/repos/monorepo/node_modules/codemirror/lib/codemirror.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/codemirror` if it exists or add a new declaration (.d.ts) file containing `declare module 'codemirror';`

1 import type { Position, Token } from 'codemirror';
                                       ~~~~~~~~~~~~

../../node_modules/@graphiql/react/types/editor/types.d.ts:1:29 - error TS7016: Could not find a declaration file for module 'codemirror'. '/Users/simen/repos/monorepo/node_modules/codemirror/lib/codemirror.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/codemirror` if it exists or add a new declaration (.d.ts) file containing `declare module 'codemirror';`

1 import type { Editor } from 'codemirror';
                              ~~~~~~~~~~~~

../../node_modules/@graphiql/react/types/editor/types.d.ts:2:52 - error TS7016: Could not find a declaration file for module 'codemirror'. '/Users/simen/repos/monorepo/node_modules/codemirror/lib/codemirror.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/codemirror` if it exists or add a new declaration (.d.ts) file containing `declare module 'codemirror';`

2 export declare type CodeMirrorType = typeof import('codemirror');

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: cd38f05a2acfafefa8c1fe24ae6b5c089cd98afa

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@graphiql/react	Patch
@graphiql/plugin-explorer	Patch
graphiql	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: SimenB / name: Simen Bekkhus (329d6e68023c2ed40abe2d9227f532e17bb31338, 9954ac297c08d2ce454bc9811c7372443dbfca67)
• :white_check_mark: login: acao / name: Rikki Schulte (285fbe2b90e423bb34ccac5b9f168dccb6b6fad5)
• :white_check_mark: login: B2o5T / name: Dimitri POSTOLOV (cd38f05a2acfafefa8c1fe24ae6b5c089cd98afa)

[andreialecu]

My concern here is that this would enforce a TypeScript-specific dependency to all users of this package, no matter if they use TS of JS. Instead I would prefer to guide users to install @types/codemirror themselves if they use TS (basically what the error message you posted says). This is something that we should definitely call out in the README.

If this is done, it should be set as an optional peer dependency. Otherwise users of yarn2/3 in PnP mode will encounter errors due to the undeclared dependency.

See: https://yarnpkg.com/advanced/rulebook/#packages-should-only-ever-require-what-they-formally-list-in-their-dependencies

[thomasheyenbrock]

Thanks for the suggestion @andreialecu! Making it an optional peer dependency sounds like the best solution for me.

[SimenB]

Yeah, same with pnpm, otherwise you rely on implementation details of hoisting.

If you go with peer dep, make sure to add it to all modules that depend on @graphiql/react as well.

---

I personally prefer adding the dependency just to not force users who don't care one bit about what your type dependencies are to add it. It would also fall apart if @graphiql/react needed another version of codemirror than one a dependent needed.

But yeah, a peer dep is way better than no dep at least 🙂

[andreialecu]

I personally prefer adding the dependency just to not force users who don't care one bit about what your type dependencies are to add it. It would also fall apart if @graphiql/react needed another version of codemirror than one a dependent needed.

+1 for this

[acao]

On top of all the good reasons for this given by @SimenB , it's also of course completely harmless to add it to dependencies from a bundling standpoint. So I generally support this approach for shipping libraries, if it matters.

Counterpoint: npm@7 (and matching pnpm releases) automatically installs peer dependencies by default again

Nice to think about: When we move to monaco editor and eventually codemirror 6, we won't need to import a @types package for the editor