Graphiql typescript migration #1242
Conversation
acao
force-pushed
the
graphiql-typescript-migration
branch
from
6aaae2d
to
374072a
Compare
ryan-m-walker
force-pushed
the
graphiql-typescript-migration
branch
from
521e24b
to
1bd4e25
Compare
|
Hi, I reached here from : https://twitter.com/rikki_js/status/1222299215126302720?s=19 How can I help? I had a look at the PR and noticed that even though it can use new TS features like optional chaining it isn't using it everywhere. If you could point me to a section that I might work on would be great (will open a PR to this branch?). Or I can just point out the things in a review first? |
|
@rohit-smpx thanks so much for helping out! thats will be great! what would probably work best would be for you to fork graphiql and open a PR against this branch on graphql/graphiql. just post PR here in comments, and then we can merge them into this working branch? if you see anywhere to make improvements such as optional chaining, or improved typings in general for graphiql thats always helpful. if you pull it up in vscode you can see what’s left, mostly the top level src/ components like GraphiQL.tsx |
|
we are keeping it as close to a refactor as possible for now, so that we can begin rearchitecting with confidence following this conversion effort |
Got it. I'll try my hand at this by tomorrow. |
|
Looks like next week is going to be much more realistic for me to dive in on this. Lots of planning this week |
|
@rohit-gohri we merged your PR, thanks for your suggestions they were very helpful and important! |
Glad to be of any help :D |
acao
force-pushed
the
graphiql-typescript-migration
branch
from
8d62451
to
329e8ae
Compare
| @@ -11,7 +11,15 @@ import React from 'react'; | |||
|
|
|||
| import onHasCompletion from '../utility/onHasCompletion'; | |||
| import commonKeys from '../utility/commonKeys'; | |||
| import { SizerComponent } from 'src/utility/CodeMirrorSizer'; | |||
|
|
|||
| declare module CodeMirror { | |||
There was a problem hiding this comment.
I was trying to fix this before, so I opened a PR in DefinitelyTyped : DefinitelyTyped/DefinitelyTyped#42194 I think this can be removed when that gets merged?
There was a problem hiding this comment.
great! thank you for that! i was too lazy to do that 😆
monaco is typescript native and will be much easier, so all of the codemirror stuff will be vanishing soon, which is one of our next big steps
acao
force-pushed
the
graphiql-typescript-migration
branch
3 times, most recently
from
6fc5029
to
9046803
Compare
- converted files to typescript
- Co-authored-by: Rikki Schulte <rikki.schulte@gmail.com>
acao
force-pushed
the
graphiql-typescript-migration
branch
from
9046803
to
d8bd509
Compare
|
@ryan-m-walker I think we're done for this pass 😆 |
For #1189 - Convert GraphiQL to Typescript with minimal production changes. True refactor is ideal.