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

[Umbrella] TypeScript Migration (Help wanted!) #21995

Open
224 of 263 tasks
blainekasten opened this issue Mar 5, 2020 · 139 comments
Open
224 of 263 tasks

[Umbrella] TypeScript Migration (Help wanted!) #21995

blainekasten opened this issue Mar 5, 2020 · 139 comments
Labels
help wanted not stale topic: TypeScript

Comments

@blainekasten
Copy link
Contributor

blainekasten commented Mar 5, 2020

Hello Community!

Gatsby is rewriting our core codebase in TypeScript! Checkout the RFC: #21798

We would love love love your help! This work is incredibly important in Gatsby being a stable product and you could be a part of it! We are looking for community members who would be willing to jump into the codebase in transition files from .js to .ts and add the proper typings. We have a few guidelines we are wanting to stick to when doing this:

Gatsby TS Guidelines

  • Only use named exports, no export default.
  • When importing a package, always try to find community maintained types instead of typing ourselves. Look at the DefinitelyTyped github repository.
  • Opt for using Interfaces for objects and functions.
  • Type aliases should be used for primitives, unions and tuples
  • Interface declaration merging is an anti-pattern and should not be used internally.
  • Use Array<type> rather than type[] syntax, as it's clearer and matches built-in types such as Map.
  • Use TypeScript's helper types, such as Record, rather than defining your own.
  • Every function should type its input and output (even if it’s inferrable)
  • Enums are great, but beware they map to index numbers if not redefined. This can cause weird problems for public APIs that interop to normal JS.
  • The any type can be used for stubbing module types that would be out of scope to migrate. But it should not be used for any functional input/outputs. The end goal of this migration is to have ZERO any types in the codebase. See if unknown makes more sense in that context. Don't use @ts-ignore comments.
  • Lastly and most imporantly, we want to have simple types whenever possible. Primarily we should try to avoid Generics. Generic code can be highly reusable, but that can also come with the greater chance of breaking things. If you do need to use generics, please either give them default values or ensure the generic value can be inferred. Code should be simple when possible. DRY is not always best.

If you're interested in getting involved, check out the list below. If you're interested in doing something, please respond as a comment, i'll put your name by a file so others know it's "claimed", when the PR is merged i'll check the box.

Tips to get involved: first read the contribution docs, and see how to set up your local environment. After you've claimed the file to work on, convert it locally and ensure there are no typing or lint errors displayed in your IDE. Run yarn typecheck and yarn lint:code in the root before opening the PR, which will show any errors. Once you're ready, open the PR and one of us will review it. Thanks, and good luck!

File list








  • packages/gatsby-page-utils
    • src/
      • /create-path.js + test
      • /ignore-path.js + test
      • /index.js
      • /validate-path.js + test
      • /watch-directory.js


@blainekasten blainekasten added help wanted status: community assigned labels Mar 5, 2020
@blainekasten blainekasten self-assigned this Mar 5, 2020
@herecydev
Copy link
Contributor

herecydev commented Mar 5, 2020

I'll happily pick up gatsby-link. Been meaning to learn some more around there!

@blainekasten
Copy link
Contributor Author

blainekasten commented Mar 6, 2020

@herecydev That sounds great! I'll put your name down! That one is relatively small and isolated, so feel free to do that whole package in one PR if you want.

@herecydev
Copy link
Contributor

herecydev commented Mar 6, 2020

@blainekasten just a few preliminary comments. There are quite a few globals/window variables: __BASE_PATH__, ___navigate, ___push, ___replace, __PATH_PREFIX__, ___loader. Is there a strategy about making those available to typescript? I believe they're all defined from within the gatsby package.

@blainekasten
Copy link
Contributor Author

blainekasten commented Mar 6, 2020

@herecydev those variables are def meant to be like "private" "secret" variables.. So I don't think we'll expose any types from gatsby package as they are only meant to be used in a few places. I think the best course of action would just be to define types for them in the gatsby-link package.

@aminkhp
Copy link

aminkhp commented Mar 6, 2020

I'm excited to contribute, which package do you suggest for starting?

@blainekasten
Copy link
Contributor Author

blainekasten commented Mar 6, 2020

@aminkhp awesome!! I would love it if we could just start chunking off individual files in PRs. Want to tackle gatsby-core-utils? I would love if you just did one file per PR as they are all very isolated.

@robertrbairdii
Copy link

robertrbairdii commented Mar 6, 2020

I'd be happy to contribute some types. I'd be able to tackle gatsby-page-utils.

I took a peek at gatsby-core-utils by the way, and it looks like true-case-path is just an empty file. I'm not sure if that's intentional, but it might be worth deleting from the repo and removing from the list? https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-core-utils/src/true-case-path.js

@blainekasten
Copy link
Contributor Author

blainekasten commented Mar 7, 2020

@robertrbairdii Great! If you could chunk up the work and submit 1 PR for each line I had, that would be fantastic.

Good point re the gatsby-core-utils. cc @aminkhp

@mottox2
Copy link
Contributor

mottox2 commented Mar 7, 2020

I'd like to try src/commands/build-javascript.js.

ADDITION: I'd also like to tackle babel-preset-gatsby

@jlkiri
Copy link
Contributor

jlkiri commented Mar 7, 2020

I spent some time in src/redux/actions/public.js before, so I think I can help with it.

@robertrbairdii
Copy link

robertrbairdii commented Mar 7, 2020

@blainekasten I was looking at the .babelrc while converting gatsby-page-utils and I noticed that there is no reference to @babel/plugin-transform-typescript in babel-preset-gatsby-package (https://github.com/gatsbyjs/gatsby/blob/master/packages/babel-preset-gatsby-package/index.js
). And running a test build, the converted .ts files don't make it to the /dist folder.

The babel config at the root of the repo has an override for it https://github.com/gatsbyjs/gatsby/blob/master/.babelrc.js#L12 And that config seems to be imported by other packages such as https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/babel.config.js

As far as I know, you can only pass the extensions as an argument through the CLI, so we would have to add the --extensions flag to each babel reference in the scripts section of all the package.json of the packages being converted. You could then either provide the override as is done in the root .babelrc or add @babel/plugin-transform-typescript in babel-preset-gatsby-package and allow values like isTSX to be configured via options passed in to the package.

I think this will block getting some of the TS conversions merged in and published.

@herecydev
Copy link
Contributor

herecydev commented Mar 7, 2020

@robertrbairdii I came to the same findings on my PR: #22027

It wasn't obvious at first, but lucky there is some prior art in the gatsby-cli project to look at. It might be worth a ticket to sweep through all the packages to support ts, which will make the files more approachable as a whole

@danielkov
Copy link
Contributor

danielkov commented Mar 7, 2020

I would love to help out and I don't have any preferences. Can you recommend me some parts to migrate?

@MichaelDeBoey
Copy link
Contributor

MichaelDeBoey commented Mar 7, 2020

@danielkov Seeing by @blainekasten's comment (#21995 (comment)), I think you could start in gatsby-core-utils

@danielkov
Copy link
Contributor

danielkov commented Mar 7, 2020

@MichaelDeBoey I also have my eyes on gatsby-cli/src/structured-errors if nobody else is on it and then move onto gatsby-core-utils in another PR. Does that sound good?

@MichaelDeBoey
Copy link
Contributor

MichaelDeBoey commented Mar 7, 2020

@danielkov That one's already taken by @LekoArts in #20597

@danielkov
Copy link
Contributor

danielkov commented Mar 7, 2020

I see #22031 is addressing one of the files of gatsby-core-utils. Should I wait for that merge, or start independently and resolve conflicts when done?

@MichaelDeBoey
Copy link
Contributor

MichaelDeBoey commented Mar 7, 2020

@danielkov You can just start working on it.
The conflicts can be fixed before merging. 🙂

@AKASHNINJA
Copy link

AKASHNINJA commented May 31, 2021

I will work on packages/babel-preset-gatsby/index.js + test

@kartikcho
Copy link
Contributor

kartikcho commented Jun 19, 2021

Any reason why packages/gatsby/src/utils/profile.js is completely commented out and if it has been unused since 2018, should we remove it altogether @blainekasten?

@alisson-suzigan
Copy link
Contributor

alisson-suzigan commented Jun 20, 2021

Hi guys, it's been a while since I've been around here.
I saw that the last attempt to migrate the schema/schema-composer.js to TS was a long time ago.
I opened a PR #31998 with this migration.

@ashiishme
Copy link
Contributor

ashiishme commented Jul 3, 2021

I would like to work on src/schema/infer/add-inferred-fields.js

@A-Scratchy
Copy link

A-Scratchy commented Jul 11, 2021

Are these still ongoing? I'd be happy to pick some up.

@lippytm
Copy link

lippytm commented Jul 11, 2021

Non-linear logic from dead body Space Aliens Ghostbusters Borg Super Artificial Intelligence NFT Crypto Time Machines Reruns Universes Reruns have been Repoed with the Harry Potter's Jumanji Educational Systems Reruns Universes Reruns and Upgradable; "But I"m usually dead 30+ years ago; but not always?

@AFEEFALISHAN
Copy link

AFEEFALISHAN commented Jul 17, 2021

hi

@nategiraudeau
Copy link
Contributor

nategiraudeau commented Aug 11, 2021

@LekoArts can I work on
packages/gatsby/src/bootstrap/load-themes/index.js,
packages/gatsby/src/schema/extensions/index.js,
packages/gatsby/src/schema/infer/add-inferred-fields.js,
and packages/gatsby/src/schema/infer/index.js?

@tonyhallett
Copy link
Contributor

tonyhallett commented Oct 19, 2021

I was considering updating CreateResolverArgs from index.d.ts whilst we wait for the js allocations to be completed ? To type this correctly it is necessary to ask a question, one that is important for those performing schema migration.

For createResolvers the field configs provided are passed to type composer methods extendField ( or addFields but the intent is the former ). As such should the typing be the same as graph-ql compose with a change to the GatsbyResolver ( or a new WrappingGatsbyResolver with info typed for the originalResolver) ? Or is gatsby not exposing the thunks from graphql-compose given that there is no mention in the docs ?

  public extendField<TArgs = any>(
    fieldName: string,
    partialFieldConfig: Partial<
      ObjectTypeComposerFieldConfigAsObjectDefinition<TSource, TContext, TArgs>
    >
  ): this;

export type ObjectTypeComposerFieldConfigAsObjectDefinition<TSource, TContext, TArgs = any> = {
  type: ThunkWithSchemaComposer<
    ComposeOutputTypeDefinition<TContext> | Resolver<any, TContext, any>,
    SchemaComposer<TContext>
  >;
  args?: ObjectTypeComposerArgumentConfigMapDefinition<TArgs>;
  resolve?: GraphQLFieldResolver<TSource, TContext, TArgs>;
  subscribe?: GraphQLFieldResolver<TSource, TContext>;
  deprecationReason?: string | null;
  description?: string | null;
  extensions?: Extensions;
  [key: string]: any;
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted not stale topic: TypeScript
Projects
None yet
Development

No branches or pull requests