Nit: InputCommonProps? or CommonInputProps?

I was copying the ___Commons convention from a few of the other converged components, for things shared between props and state. So I'd probably prefer to keep it as-is, but I don't feel super strongly about it.

Question - when moving to React hooks, many teams choose to move from interface to type declarations for their props and use type intersections over extends. Are interfaces giving us something that types would not? When I see interfaces I always think a class will implement it.

IIRC we discussed this but didn't come to a solid conclusion about which convention we wanted (and the discussion apparently happened before we started documenting things in RFCs). There may be some specific reason that type is commonly used for slots, but I can't remember offhand. @layershifter do you remember?

There may be some specific reason that type is commonly used for slots, but I can't remember offhand. @layershifter do you remember?

When TS emits type declarations sometimes it inlines type declarations (microsoft/TypeScript#37151), so in general interfaces are preferred for in case.

We have seen extreme cases where this duplication has inflated declaration files from 7KB to 700KB. That’s quite a lot of redundant code to download and parse.

Through experimentation, we discovered potential techniques to prevent inlining of type declarations, such as:

• prefer interface instead of type (interfaces are not inlined)
• If an interface needed by a declaration is not exported, tsc will refuse to inline the type and will generate a clear error (e.g., TS4023: Exported variable has or is using name from external module but cannot be named.)

https://www.techatbloomberg.com/blog/10-insights-adopting-typescript-at-scale/

Interesting, thanks! Incidentally this diff (the old version) shows an example of expanded types. 😬 https://github.com/microsoft/fluentui/pull/19376/files#diff-805da48a70295edd847bed2a0dda92910c21f9f0185fa238bc342d167ae4407b

😢

Is it expected to have spans there? Why not divs?

It's intentional (there's a comment about this in renderInput). If the input is inline within a p there will be nesting errors if I use div.

In a previous version of the implementation I used div by default and span if inline was true, but someone suggested just using span always since it (probably?) doesn't make much practical difference (all the elements either have display explicitly set, or are within a flex layout). I'm open to suggestions here though, and could go back to that if there's some reason it would be better.

Is it the same as react-text/src/typographyStyles/typographyStyles.ts? Should we reuse typographyStyles?

It's similar but doesn't quite cover all the cases--the design for Input specifies large as using 400 size but I'm pretty sure it still uses regular font weight. Though it's possible that was an oversight. (Incidentally...WHY did we have to use numeric size names that look like font weights...?? I know this is more of a complaint for design and probably too late. 😞)

Also the individual typography styles actually aren't exported at the moment. Should we change that (@andrefcdias)? In this case it's not feasible/desirable to use the full text components like Body. (Either way, since it's a change in a separate package I think I'll leave this as-is for this PR.)

I don't think that this typing is correct as React.FunctionComponent does not have ref on it's interface:

Type '{ foo: string; ref: RefObject<unknown>; }' is not assignable to type 'IntrinsicAttributes & Props & { children?: ReactNode; }'.
  Property 'ref' does not exist on type 'IntrinsicAttributes & Props & { children?: ReactNode; }'.

TypeScript playground

InputProps has ref through inheritance. I strongly prefer an explicit type for the component here because it makes the public API cleaner.

Not necessarily something to change now and in this PR, but in the future I think all our storybook examples should be accessible by default (i.e. have a visible label and no placeholder), with potentially one example of less-accessible variations like not having a visible label.

Yup I plan to follow up on this when I make a more finished version of the stories