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

Add Documentation for Using Hooks with TypeScript #1467

Merged
merged 1 commit into from
Mar 20, 2021

Conversation

ITenthusiasm
Copy link
Member

What Would You Like to Add/Fix?

This is a PR to add documentation for using hooks with TypeScript. I didn't expect 10.6.0 to get released this fast. But since it's out there, I think getting these docs out as soon as possible is optimal. It will help people who've been waiting for better TS support, and it will also give better ideas on how to migrate older code.

Tests are added under a docs.tsx file. If the examples are simple, then any TS code that gets added to the documentation should probably have something like this. Implicit types are complicated, so it's important to know they work as expected before suggesting the change to other users.

@ITenthusiasm ITenthusiasm requested a review from kof as a code owner March 14, 2021 12:15
@ITenthusiasm
Copy link
Member Author

Although this is urgent, please do closely observe the documentation. We should make sure the docs are at least decent before publishing any changes.

@ITenthusiasm ITenthusiasm force-pushed the feat/docs-hooks-typescript branch 2 times, most recently from 065f216 to a238ff6 Compare March 14, 2021 12:23
@ITenthusiasm
Copy link
Member Author

Added some amend commits. The first one was to make sure the code examples in the docs.tsx test were properly updated in the documentation. The second commit simplified some phrasing.

@ITenthusiasm
Copy link
Member Author

After this gets merged, it should be simple enough to tackle #1459 in a separate PR

@kof
Copy link
Member

kof commented Mar 14, 2021

Maybe it makes sense to have a page for usage with typescript, not just for hooks api, but everything related to ts and react-jss and move it to a separate md file

@ITenthusiasm
Copy link
Member Author

Maybe it makes sense to have a page for usage with typescript, not just for hooks api, but everything related to ts and react-jss and move it to a separate md file

That could definitely be useful. I'd be happy to help with everything related to the TS + ReactJSS portion. But I'm unfamiliar with the other packages, so I wouldn't be able to say much more there.

With respect to React-JSS, since HOCs are deprecated, it would be a little strange to have TS examples with them. The only TS example I'll have in that area is the TS example of creating a lazy migration HOC. (Technically speaking, it may be worth considering exporting that lazy migration HOC.) I don't know if there's much else TS to show for React-JSS.

@ITenthusiasm
Copy link
Member Author

Any thoughts about the content in the PR so far? Should it be changed at all?

@kof
Copy link
Member

kof commented Mar 15, 2021

With respect to React-JSS, since HOCs are deprecated, it would be a little strange to have TS examples with them.

Totally, I would only document APIs we want people to use. I thought about ThemeProvider, JssProvider, global Theme and probably some other things, I wold just go over the main document for react-jss and see what needs TS docs/examples

And I would move this to a react-jss-ts.md file and link it from multiple places where people would look for them as well as we need to add it to navigation structure on the documentation site

@kof
Copy link
Member

kof commented Mar 15, 2021

Once ready mabe ping @cssinjs/typescript for a review

@ITenthusiasm
Copy link
Member Author

@cssinjs/typescript Any thoughts on the additions to the TypeScript section?

@ITenthusiasm ITenthusiasm mentioned this pull request Mar 15, 2021
2 tasks
@ITenthusiasm
Copy link
Member Author

@k-yle, it doesn't look like you're in @cssinjs/typescript, but you gave some really helpful feedback on #1460. Would appreciate your comments here as well.

@ITenthusiasm
Copy link
Member Author

ITenthusiasm commented Mar 15, 2021

Also I missed this comment:

Totally, I would only document APIs we want people to use. I thought about ThemeProvider, JssProvider, global Theme and probably some other things, I wold just go over the main document for react-jss and see what needs TS docs/examples

And I would move this to a react-jss-ts.md file and link it from multiple places where people would look for them as well as we need to add it to navigation structure on the documentation site

I'll look into this when I have the time.


### Implicit Typing

If you want to provide `Props` and/or `Theme` types without having to supply the `RuleNames` type, you can make TypeScript infer the types implicitly:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option that may be worth documenting:

If CustomTheme is the same across your whole application, TypeScript and JavaScript users can create a file called global.d.ts anywhere in their project. It is not imported by anything; TypeScript and/or VSCode's TSServer finds it automatically. It could like this:

declare global {
  namespace Jss {
    export interface Theme {
      themeColour: string;
    }
  }
}

export {};

The benefit is that you don't need to specify CustomTheme when you call createUseStyles. There is a bit more info in #1453

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I was thinking about that, but I wasn't sure how to introduce it. What you provided gave me a good amount of inspiration though. Thanks!

}
```

You only need to supply as much type information as you need. For instance, if you aren't using a `Theme`, then you don't need to supply a type for it. You'll notice that if you want to supply types for `Props` or `Theme`, you'll have to supply the type for `RuleNames` first. This means that if you want autocomplete for the `classes` that come from `useStyles`, you'll need to explicitly provide the strings you'll be using. This restriction is due to a limitation in TypeScript [that will hopefully get resolved soon](https://github.com/microsoft/TypeScript/pull/26349).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would consider extending the last sentence to say "or, see the next section for a workaround".

Just a thought, in case some people stop reading at the end of that section. There seems to be quite a misunderstanding about this point (#1460 (comment), #1469)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's a good point. I didn't expect this to be such a large source of confusion. I tried to add some additional words like you said for clarity.

My recent commit has changes that address both of your suggestions. If you have free time, let me know what you think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great 👍

@ITenthusiasm
Copy link
Member Author

Totally, I would only document APIs we want people to use. I thought about ThemeProvider, JssProvider, global Theme and probably some other things, I wold just go over the main document for react-jss and see what needs TS docs/examples

@kof It would definitely be nice to have TS documentation for those other guys. Unfortunately, I don't have much familiarity with them quite yet -- particularly ThemeProvider and JssProvider. (If they're just React components, then they hopefully wouldn't need much documentation, though -- if any.) At the same time, some information on how to use the hooks would certainly be helpful to have soon. It would help people who are starting to use TS, as well as anyone who needs to perform a migration or who wants to use the new changes.

After the existing documentation changes get approved, it could be good to launch out what we have now. Or is that too hasty? Thoughts?

And I would move this to a react-jss-ts.md file and link it from multiple places where people would look for them as well as we need to add it to navigation structure on the documentation site

Are you imagining a navbar that looks something like this?

React JSS

  • API
  • TypeScript

I'd be fine with extracting out the current changes into their own file if that's desirable. However, I don't know how to make nested links appear in the navbar on the website.

@ITenthusiasm ITenthusiasm force-pushed the feat/docs-hooks-typescript branch from 1735461 to 7a0ed12 Compare March 16, 2021 20:23
@@ -587,4 +587,121 @@ const Button = () => {

## TypeScript

TODO hooks support
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets move changes to react-jss-ts.md? So that we have space for all TS specific things and link that file from react-jss.md and other places where we find the right fit. Good places to link is super important for discoverability!

@kof
Copy link
Member

kof commented Mar 20, 2021

We can merge what we have now and add more later

@ITenthusiasm
Copy link
Member Author

@kof I moved the TS info to its own page as requested and squashed all the commits. Unless there are any other desired changes, this PR is ready to merge.

As I mentioned previously, I don't know how to make the TypeScript portion appear as a sub link under React-JSS (similar to how API and Styles Syntax are sub links under JSS Core). That would likely be good to have though.

@kof
Copy link
Member

kof commented Mar 20, 2021

Maybe for now it would be good to link from the current md pages to the TS document?

@kof
Copy link
Member

kof commented Mar 20, 2021

What about creating a "## Using with TypeScript" section in react-jss.md and a sentence and a link. Section because this way it would be much more visible than a link within a block of text anywhere

* Created a new page for using React-JSS with TypeScript.
* Added documentation for using hooks with TypeScript.
* Added documentation for creating a global Theme type.
* Added tests for verifying that the examples in the docs behave well.
@ITenthusiasm ITenthusiasm force-pushed the feat/docs-hooks-typescript branch from c368a6d to cf0449f Compare March 20, 2021 17:34
@ITenthusiasm
Copy link
Member Author

ITenthusiasm commented Mar 20, 2021

What about creating a "## Using with TypeScript" section in react-jss.md and a sentence and a link. Section because this way it would be much more visible than a link within a block of text anywhere

Pushed those new changes to react-jss.md. You can let me know if they're sufficient or not.

@kof kof merged commit 39bae76 into cssinjs:master Mar 20, 2021
@kof
Copy link
Member

kof commented Mar 20, 2021

merged, thank you!

@ITenthusiasm ITenthusiasm deleted the feat/docs-hooks-typescript branch March 21, 2021 01:36
@kof
Copy link
Member

kof commented Jun 27, 2021

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.

3 participants