[docs] Add paragraph on withStyles with multiple classes #9851

clentfort · 2018-01-12T20:57:43Z

Summary:

Add a paragraph to the TypeScript specific documenation of withStyles that explains how to type components receiving multiple classes from withStyles.

**Summary**: Add a paragraph to the TypeScript specific documenation of `withStyles` that explains how to type components receiving multiple classes from `withStyles`.

mbrookes

Thanks for your PR! I can't comment on the technical content, so just some suggestions on style. We're gradually sharpening up the docs to follow technical writing guidelines, but we still have a long way to go!

mbrookes · 2018-01-12T21:32:56Z

docs/src/pages/guides/typescript.md

@@ -94,6 +94,44 @@ const DecoratedNoProps = decorate<{}>( // <-- note the type argument!

 To avoid worrying about this edge case it may be a good habit to always provide an explicit type argument to `decorate`.

+### Injecting Multiple Classes
+
+TypeScript does not support variadic types, therefore there is no clean way to type usage of `withStyles` that injects multiple classes into a component. There is a hacky workaround that allows you to get support for classes inside your component while only exposing the props you define.


"However, there is a hacky workaround"

mbrookes · 2018-01-12T21:33:22Z

docs/src/pages/guides/typescript.md

+
+TypeScript does not support variadic types, therefore there is no clean way to type usage of `withStyles` that injects multiple classes into a component. There is a hacky workaround that allows you to get support for classes inside your component while only exposing the props you define.
+
+Take the following code as an example: We define our own props in the `Prop`-type, while the props the component actually receives is a union of our `Prop`-type and a `WithStyles`-type for each respective style we defined (`one` and `two`). To prevent TypeScript from exposing the props set through `withStyles` to the consumers of our component, we cast it to a `React.ComponentType<Props>` before exporting it. 


"Take the following code for example."

Also, could you perhaps rephrase this without the personal pronoun "we"?

oliviertassinari · 2018-01-12T22:35:37Z

docs/src/pages/guides/typescript.md

+   someProp: string;
+};
+
+type PropsWithStyles = Props & WithStyles<"one"> & WithStyles<"two">;


I was expecting:

type PropsWithStyles = Props & WithStyles<'one' | 'two'>;

Oh wow, this is an even better solution. Thanks for pointing this out. I played around a bit and looked at the types, but never thought about using a |.

clentfort · 2018-01-12T22:45:04Z

I will update tomorrow with @oliviertassinari suggestion and update the text accordingly. Thanks for pointing out this solution.

pelotom · 2018-01-12T23:14:29Z

docs/src/pages/guides/typescript.md

@@ -94,6 +94,44 @@ const DecoratedNoProps = decorate<{}>( // <-- note the type argument!

 To avoid worrying about this edge case it may be a good habit to always provide an explicit type argument to `decorate`.

+### Injecting Multiple Classes
+
+TypeScript does not support variadic types, therefore there is no clean way to type usage of `withStyles` that injects multiple classes into a component. However, there is a hacky workaround that allows you to get support for classes inside your component while only exposing the props you define.


I don't understand this paragraph, or to the extent that I do, I don't think it's accurate. It's certainly possible to inject multiple classes into a component.

mbrookes · 2018-01-13T12:29:23Z

docs/src/pages/guides/typescript.md

@@ -94,6 +94,42 @@ const DecoratedNoProps = decorate<{}>( // <-- note the type argument!

 To avoid worrying about this edge case it may be a good habit to always provide an explicit type argument to `decorate`.

+### Injecting Multiple Classes
+
+Injecting multiple classes into a component is straightforward possible. Take the following code for example: The classes `one` and `two` are both available with type information on the `classes`-prop passed in by `withStyles`.


"is as straightforward as possible" (add 2 x "as")

Regarding "Take the following code for example:" it should be something other than a colon here, as the example doesn't immediately follow, but I can't decide if it should be a full stop (period), or a hyphen, running on to the next clause. Take your pick, either will work.

Sorry for being picky. 😈

No problem. Just please no more edits.

Or just edit the PR directly. :D

clentfort · 2018-01-13T13:53:50Z

Sorry for the ninja edit after your approve. There still was a typing error.

mbrookes · 2018-01-13T14:47:39Z

@clentfort NP, I was only approving my requested text changes. I'll let someone who knows TS review the code samples.

pelotom

My honest feeling is that this section doesn't add much; I'd rather just augment the previous section so that it has more than one class. That said, I don't object to it being merged.

[docs] Add paragraph on withStyles with multiple classes

8672985

**Summary**: Add a paragraph to the TypeScript specific documenation of `withStyles` that explains how to type components receiving multiple classes from `withStyles`.

oliviertassinari requested a review from pelotom January 12, 2018 21:39

oliviertassinari added the docs Improvements or additions to the documentation label Jan 12, 2018

oliviertassinari requested a review from sebald January 12, 2018 21:40

oliviertassinari added the typescript label Jan 12, 2018

mbrookes requested changes Jan 12, 2018

View reviewed changes

oliviertassinari reviewed Jan 12, 2018

View reviewed changes

Update typescript.md

44847a0

pelotom reviewed Jan 12, 2018

View reviewed changes

Update typescript.md

7fe7266

mbrookes requested changes Jan 13, 2018

View reviewed changes

Update typescript.md

83314eb

mbrookes approved these changes Jan 13, 2018

View reviewed changes

Update typescript.md

45aff7a

oliviertassinari approved these changes Jan 13, 2018

View reviewed changes

pelotom approved these changes Jan 13, 2018

View reviewed changes

oliviertassinari merged commit 24f5b01 into mui:v1-beta Jan 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[docs] Add paragraph on withStyles with multiple classes #9851

[docs] Add paragraph on withStyles with multiple classes #9851

clentfort commented Jan 12, 2018

mbrookes left a comment •

edited

mbrookes Jan 12, 2018

mbrookes Jan 12, 2018

mbrookes Jan 12, 2018 •

edited

oliviertassinari Jan 12, 2018

clentfort Jan 12, 2018

clentfort commented Jan 12, 2018

pelotom Jan 12, 2018

mbrookes Jan 13, 2018

clentfort Jan 13, 2018

clentfort Jan 13, 2018

clentfort commented Jan 13, 2018

mbrookes commented Jan 13, 2018

pelotom left a comment


		TypeScript does not support variadic types, therefore there is no clean way to type usage of `withStyles` that injects multiple classes into a component. There is a hacky workaround that allows you to get support for classes inside your component while only exposing the props you define.

		Take the following code as an example: We define our own props in the `Prop`-type, while the props the component actually receives is a union of our `Prop`-type and a `WithStyles`-type for each respective style we defined (`one` and `two`). To prevent TypeScript from exposing the props set through `withStyles` to the consumers of our component, we cast it to a `React.ComponentType<Props>` before exporting it.

[docs] Add paragraph on withStyles with multiple classes #9851

[docs] Add paragraph on withStyles with multiple classes #9851

Conversation

clentfort commented Jan 12, 2018

mbrookes left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbrookes Jan 12, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clentfort commented Jan 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clentfort commented Jan 13, 2018

mbrookes commented Jan 13, 2018

pelotom left a comment

Choose a reason for hiding this comment

mbrookes left a comment •

edited

mbrookes Jan 12, 2018 •

edited