Fluentui: Can't seem to style Button through 'subComponentStyles' in IProcessedStyleSet<>

[redoz]

Now that #6061 landed I can no longer apply styling to Buttons through the subComponentStyles property on my IProcessedStyleSet<> without jumping through extra hoops:

If I do this:
styles={this._classNames.subComponentStyles.button}
I get an error:

Type 'IStyleFunction<{}, IButtonStyles> | (() => Partial<IButtonStyles>)' is not assignable to type 'IButtonStyles | undefined'.
  Value of type 'IStyleFunction<{}, IButtonStyles>' has no properties in common with type 'IButtonStyles'. Did you mean to call it?

So I try to call it:
styles={this._classNames.subComponentStyles.button({} /* empty as there is no IButtonStyleProps */)}
or like this:
styles={this._classNames.subComponentStyles.button()}

and I get a different error:

[ts] Cannot invoke an expression whose type lacks a call signature. Type 'IStyleFunction<{}, IButtonStyles> | (() => Partial<IButtonStyles>)' has no compatible call signatures.

So it seems I need some kind of construct like this now:
styles={(this._classNames.subComponentStyles.button as (IStyleFunction<{}, IButtonStyles>))({})}

But if I read the types correctly couldn't IStyleFunction<IStyleProps, IStyles> | (() => Partial<IStyles>) be made more succinct if IStyleFunction was defined like this (with "props" being optional):

export declare type IStyleFunction<TStylesProps, TStyleSet extends IStyleSet<TStyleSet>> = (props?: TStylesProps) => Partial<TStyleSet>;

But this might have consequences elsewhere, and I haven't yet tried to apply the change to see what those might be so it might not be the way to go. Though, if the props argument really is optional perhaps it is a suitable fix, and if it isn't, could we remove (() => Partial<IStyles>) from the type definition? I'm not too familiar with the internals here so not sure why this definition looks the way it does in the first place, but maybe there is a good reason I'm not seeing?

[Nimelrian]

I noticed this as well after pulling the latest release. My test suite sadly didn't include a Button. Button defines the styles prop as only a IButtonStyles, which seems strange. Multiple other components do this as well (SpinButton, ComboBox, ...). General reminder to unify the typings...

The issue in general seems strange, because the original IProcessedStyleSet mapped every subComponentStyles property to a function as well, which e.g. the Button happily accepted.

I'm taking a closer look into the difference between the output of the original typing and the new one...

[redoz]

Before this I always manually invoked the function <IconButton iconProps={{iconName: 'ChromeClose'}} styles={this._classNames.subComponentStyles.closeButton() as IButtonStyles} /> but that might have been "wrong" as well.

[Nimelrian]

@redoz Did your code work before the release?

I just did a checkout of the old code and using a button subcomponent style causes a typing error there as well:

[ts] Type 'IStyleFunction<any, IStyleSet<any>>' has no properties in common with type 'IButtonStyles'.

So instead of the PR causing a regression it seems that the underlying issue is the fact that Button does (to the outside) not accept an IStyleFunction.

[redoz]

Let's put it like this, previously I did this:
styles={this._classNames.subComponentStyles.closeButton() as IButtonStyles}
in order to style Buttons. With this change, that method broke. It seems TypeScript is unable to resolve the type IStyleFunction<{}, IButtonStyles> | (() => Partial<IButtonStyles>) as something callable, even though both sides of the pipe are callable types. So now I'm forced to explicitly cast it into a IStyleFunction (e.g. (props) => styleset) or () => styleset in order to invoke it.

The way I look at that definition is "We have a function that has a props parameter of IStyleProps, or we have a function that takes zero arguments".

I don't see how the caller would know which one it is, so the end result would seem to be that we have a function with an optional props parameter. So rather than forcing an arbitrary cast (with or without props) and invoking it, we would just change the type definition to either make the props explicitly optional, or force it to be required by dropping the | () => IStyleSet<T> branch.

[micahgodbolt]

Need to dig into this a bit more, but just a reminder that the Button styles interface hasn't caught up with our current approach using IStyleFunctionOrObject. This is probably why things are working as expected with Button. Converting Button over is high on our priorities, but we want to make sure we do it right, so it's going to take time.

[micahgodbolt]

Okay, I was able to confirm that subComponentStyles won't work on Button just yet. The interfaces aren't compatible as subComponentStyle always converts the styles into a style function.

I know this is a painpoint as button is probably the first subComponent we want to be able to style!

[cliffkoh]

The workaround where you directly invoke the subcomponent style function should be fine for dealing with Button's inability to take in a style function today.

Where did IStyleFunction<{}, IButtonStyles> | (() => Partial<IButtonStyles>) come from? After the merge of #6061, I would think that IProcessedStyleSet would now always deterministically have either an IStyleFunction or a () => Partial but not both.

Would it be possible to include a standalone repro and also include typescript version?

For the inconvenience you're hitting, I think removing ()=>Partial<IButtonStyles> or making it consistent would help, but it isn't obvious to me where it is coming from...

[cliffkoh]

Oh I can repro it now. Digging in!

[micahgodbolt]

thanks @cliffkoh! If you can find a way to add support for the button interface, that'll unlock a TON of usecases.

Also, you can keep an eye on the Button conversion over at #6134 (not much there to see yet).

[redoz]

I'll try to setup a minimal repro tomorrow when I'm back at work (timezones and all), and I'm using TypeScript 3.0.1 if memory serves, but I'll verify tomorrow.

Edit: Never mind, I see you have a repro already, thanks for taking a look.

[cliffkoh]

No worries, I can repro it now and I know what caused it. It wasn't in @Nimelrian's initial PR but this happened because of the change @Nimelrian did at my request as the initial version did not handle style objects correctly.

Minimally, we should be able to make the 2nd part ()=>Partial<IButtonStyles> optionally take arguments that it disregards so you should then be able to invoke it with this.classNames.subComponentStyles.button({}).

[cliffkoh]

I will submit a PR to fix :)

[Nimelrian]

@cliffkoh

Where did IStyleFunction<{}, IButtonStyles> | (() => Partial) come from?

IProcessedStyleSet now maps subComponentStyles like this:

type TrimToFunction<T> = T extends (...args: any[]) => any ? T : () => T;

// ...

subComponentStyles: {
  [P in keyof TStyleSet['subComponentStyles']]: TrimToFunction<TStyleSet['subComponentStyles'][P]>
};

So when the property is typed as IStyleFunction<ISomeStyleProps, ISomeStyles> (Edit: Or any kind of function, for that matter...), it keeps that typing, since the extends clause condition hits and T is automatically inferred.

When the property is typed as IPartial<ISomeStyles> the extends clause condition fails and thus the type is mapped to () => IPartial<ISomeStyles>.

Now, when the property is typed as IStyleFunctionOrObject<ISomeStyleProps, ISomeStyles>, from how I understand conditional types, each type of the underlying union is individually evaluated. So you get a combination / union of the processes / types depicted above.

@redoz How is the subComponentStyles prop of your Style type typed? As an Partial<IButtonStyles>, or IStyleFunctionOrObject<{}, IButtonStyles>?

[Nimelrian]

@cliffkoh If you're doing a PR, changing the "else" type expression of TrimToFunction to (...args: any[]) => T should do the trick.

[cliffkoh]

That is my plan 😄✋

[redoz]

@Nimelrian pretty sure i used IStyleFunctionOrObject<, > as I think I saw that used somewhere. But I would have to double check tomorrow to be sure. If current recommendation is to use something else I'll gladly change it on my end?

[cliffkoh]

@Nimelrian Hmm it doesn't seem to do the trick...

Link here.

[Nimelrian]

@redoz Ok, that would probably be invalid, because Button cannot take IStyleFunctions as of right now (according to its typings).

@cliffkoh Hmm, strange, but seems like it is intended(?) according to https://github.com/Microsoft/TypeScript/issues/4612.

I'll have to take a deeper look into this tomorrow. Maybe I still can tinker a solution together using the playground today, but I doubt it.

[cliffkoh]

@Nimelrian I found a way to get it to work :) It is passing some newly written test cases I wrote. Creating the PR shortly!

[Nimelrian]

I think I've found a solution, give me a sec. Can't verify it using Fabric right now...

@cliffkoh
Can you check what happens when you change the type of TrimToFunction to this:

type Params<T> = T extends (...args: infer U) => any ? U : never;
type TrimToFunction<T> = T extends (...args: any[]) => any ? T : (...args: Params<T>) => T;

See the playground here

[cliffkoh]

What I came up with is this:

export type ReduceToFunction<T> = T extends (...args: any[]) => any ? T : never;

export type MapToFunctionType<T> = [T] extends [IStyleFunctionOrObject<any, any>]
  ? ReduceToFunction<T>
  : (...args: any[]) => T;

[Nimelrian]

Hmm, that should work, but I feel like this would again lead to a union of different function types. Strange.

Would you mind trying my solution as well using your test suite? Imho it looks a bit cleaner and seems a tiny bit easier to understand.

[cliffkoh]

Yes I will try both. If you're still around I'll let you know shortly. I am fleshing out the test cases at the moment.

[cliffkoh]

BTW my solution does not lead to a union of different function types - the square brackets is an opt-out of that.

From: https://blogs.msdn.microsoft.com/typescript/2018/03/27/announcing-typescript-2-8/

[Nimelrian]

Ah yes, I remember reading that part 😅

Yeah, conditional types are really powerful, but a little finicky at times.

[cliffkoh]

@Nimelrian your proposed solution does not work - at least not on Typescript 2.8.2 (which is what Fabric is on).

Neither does mine work perfectly, but if I were to flip the condition around - all my test cases pass in 3.0.1 (but not 2.8.2)

export type ReduceToFunction<T> = T extends (...args: any[]) => any ? T : never;

export type MapToFunctionType<T> = [T] extends [IStyleSet<any>] ? (...args: any[]) => T : ReduceToFunction<T>;

This works in Typescript 3.0.1, but does not work perfectly in 2.8.

This is because, strangely,

type Foo= IStyleFunction<any, any> extends IStyleSet<any> ? true : false;

returns true in Typescript 2.8.2 (whereas in Typescript 3, it returns false).

Looks like a bug with conditional types that was fixed in the newer Typescript version.