-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Description
Overview
All existing interfaces and types in all v7 packages are currently prefixed with I. All interfaces and types in v0 packages are not. Let's choose one path.
Reasons for prefixing
- Seeing an I prefix gives the developer ease of mind that adding the graph edge will not impact bundle size.
- Avoids common naming collisions. E.g. the interface for an
IThingcould be implemented by aThing. Without the prefix, you would need an alternative prefix or suffix to avoid the collision such asThingAPIorThingInterface, which leads to inconsistencies.
If we choose to embrace I-prefix, we can avoid naming collisions but will need to cov
Reasons against prefixing
- Consistency with the community. TypeScript coding conventions suggest avoiding. Most OSS TypeScript projects seem to follow this guidance and avoid prefixing.
- Tools like bundle-size can tell you fairly soon if an import has cost or not (but only on a per import level and not on a per type.)
- TypeScript “type” definitions aren’t technically interfaces so prefixing with the letter I feels weird.
- Linting defaults to avoiding.
- Interfaces technically can be replaced with classes and still be implmeented.
If we choose to abandon I-prefix, we need to propose naming guidance for when developers hit the classic IFoo interface vs Foo implementation.
Prior considerations
TypeScript thread here:
microsoft/TypeScript-Handbook#121
There are multiple customer complaints that the guidance has no principles or reasoning behind the decision other than it was deemed "an unnecessary relic of C# days". No guidance been provided in regards to naming conventions to deal with typing differentiation. Additionally the TypeScript team has mentioned not prefixing generic placeholders with T (e.g. function foo<TThing>(arg: TThing)).
Stack overflow article here:
Some clarifications offered:
- Don't have an
IFootype and aFooimplementation. That's "bad" becauseFooshould be more specific. - Prefixing interfaces with
Iviolates encapsulation of the type (changing from an interface to a type would then remove theIright?) Our approach has been to prefix alltypeandinterfacenames withI, and treat them interchangeably.
Proposed direction
For consistency with TypeScript conventions, we abandon I-prefix. This means that all interfaces would need to be renamed, with I-prefixed deprecated re-exports.
Example: IColor interface, or ISize type becomes:
export interface Color { ... }
export type Size = 'foo' | 'bar';
/** @deprecated - `IColor` has been deprecated, use the `Color` interface instead. **/
export interface IColor extends Color {}
/** @deprecated - `ISize` has been deprecated; use the `Size` type instead. **/
export type ISize = Size;Once we settle, we will update the wiki here:
https://github.com/microsoft/fluentui/wiki/Coding-Style#prefix-interfaces-and-sometimes-types-with-i
Special considerations
There are many scenarios where removing the prefix creates a symbol collision. Naming is hard, and removing the prefix makes things harder, so we need to have strong guidance on improved type naming conventions to reduce inconsistencies.
| Collision scenario | Example of conflict | Proposed replacement |
|---|---|---|
| componentRef interfaces | IButton |
Append ImperativeHandle suffix. (ButtonImperativeHandle) |
| fallback scenarios | IButtonStyles |
Append Type suffix. (ButtonStylesType) |
More examples of collisions in the codebase currently:
IExampleGroup, IRefObject, IRectangle, IAppCustomizations, IColorClassNames, IAnimationStyles, IAnimationVariables, ICustomizations, ISettings, ISettingsFunction, ICustomizerContext, ISelection, IDonutChart, ILineChart, IPieChart, IStackedBarChart, IVerticalBarChart, IDatePickerStyles, IAppDefinition, IAccordion, IButtonSlots, IButtonTokens, IButtonStyles, IChicletCard, ICollapsibleSection, IFloatingSuggestions, IMicroFeedback, IMicroFeedbackTokens, IMicroFeedbackStyles, IPersonaCoinStyles, ISelectedItemsList, ISliderStyles, IToggleTokens, IToggleStyles, IScrollContainer, IBreadcrumbStyles, IButtonStyles, ICalloutContentStyles, ICheckStyles, ICheckboxStyles, IColorPickerStyles, IComboBoxStyles, ICommandBarStyles, IContextualMenuItem, IContextualMenuStyles, IDatePickerStyles, IDetailsColumnStyles, IDetailsHeaderStyles, IDetailsListStyles, IDetailsRowStyles, IDetailsRowCheckStyles, IDialogContentStyles, IDialogFooterStyles, IDocumentCardStyles, IDropdown, IDropdownStyles, IBaseExtendedPicker, IFacepileStyles, IGroupHeaderStyles, IExpandingCard, IExpandingCardStyles, IImage, IKeytip, ILabelStyles, ILinkStyles, IPage, IMessageBarStyles, IModalStyles, INavStyles, IOverlayStyles, IPanelStyles, IPersonaStyles, IPersonaCoinStyles, IBasePicker, IBasePickerStyles, IPivotStyles, IProgressIndicatorStyles, IRatingStyles, IScrollablePaneContext, ISearchBoxStyles, IBaseSelectedItemsList, ISeparator, ISliderStyles, ISpinButtonStyles, IColorPickerGridCellStyles, ITeachingBubbleStyles, ITextStyles, ITextFieldStyles, IToggleStyles, IViewport, IDragDropHelper, IPosition, ISelectionZone, ICard, ICardTokens, ICardStyles, IActionableSlots, IActionable, IActionableTokens, IActionableStyles, IMenuButtonSlots, IMenuButton, IMenuButtonTokens, IMenuButtonStyles, ISplitButtonSlots, ISplitButton, ISplitButtonTokens, ISplitButtonStyles, IVerticalPersonaStyles, IVerticalPersonaTokens, ISelectedPeopleList, IExampleGroup, IChoiceGroupOptionStyles, IColorRectangleStyles, IColorSliderStyles, IPlainCard, IPlainCardStyles, ISuggestions, ISuggestionsStyles, ITagItemStyles, IStackItemStyles, ICardItem, ICardItemTokens, ICardItemStyles, ICardSection, ICardSectionTokens, ICardSectionStyles
Conversion plan for codebase
A ts-morph script would manage the renames in bulk. The process would be that for each interface identified that matches an I prefix, apply a rename across the repo. This applies to both interface and type definitions. The old name would also be exported as deprecated to avoid breaking changes:
Before:
export interface IButton { ... }
export type IButtonSize = "small" | "medium";
export interface IButtonProps {
componentRef?: RefObject<IButton>;
size?: IButtonSize;
}After:
export interface ButtonRef { ... }
export type ButtonSize = "small" | "medium";
export interface ButtonProps {
componentRef?: RefObject<ButtonRef>;
size?: ButtonSize;
}
/** @deprecated - Use `ButtonRef` instead. */
export interface IButtonRef extends ButtonRef {}
/** @deprecated - Use `ButtonSize` instead. */
export type IButtonSize = ButtonSize;
/** @deprecated - Use `ButtonProps` instead. */
export interface IButtonProps extends ButtonProps {}Conversion plan for customers
An upgrade-fluentui command would be provided for customers as an executable node script upon install. This script would apply all symbolic renames defined in a JSON file to the customer's source files when executed.
The JSON file would define all symbolic renames resulting from the automated conversion above.
When the user runs upgrade-fluentui in their repo, these changes would be applied, as an example:
Before (src/**/App.tsx)
import { Button, IButtonProps } from '@fluentui/react';
const buttonProps: IButtonProps = {
size: 'small'
};
const App = () => (
<Button {...buttonProps } />
);After:
import { Button, ButtonProps } from '@fluentui/react';
const buttonProps: ButtonProps = {
size: 'small'
};
const App = () => (
<Button {...buttonProps } />
);This process would need to be documented, and ideally mentioned when yarn installing, similar to how some npm libraries have postinstall steps that warn about out of date dependencies.