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

feat: enable @internal api stripping in all packages and fix export maps #25319

Closed

Conversation

Hotell
Copy link
Contributor

@Hotell Hotell commented Oct 20, 2022

Pre - requirements (merged PRs):

Current Behavior

  • APIs marked as @internal are being shipped to customers (TS types)
  • some packages are missing or contain invalid export maps

New Behavior

  • APIs marked as @internal are not being shipped to customers (TS types)
  • all packages contain valid export-maps

Internal Tooling Breaking Changes

  1. Generating api.md locally:

Before: yarn workspace <project-name> build:local
After: yarn workspace <project-name> generate-api

Related Issue(s)

🚨 Blockers / internal api leaks

After migration and internal api striping tooling activation, following issues were exposed

💡 API's marked as @internal leaking to public

Action needed:

  • all issues fixed

How to fix those (multiple options):

  1. remove @internal annotations from APIs that are causing the failures (not ideal as it defeats the purpose)
  2. encapsulate internal API's with non internal abstraction (not sure if possible in all cases)
  3. inline type annotation per library ( ideal solution as it enforces better domain boundaries. it will add some amount of duplication though but because TS is structural we will still get proper type safety guarantees )

Summary of internal API (tokens) leaking grouped by package

@fluentui/react-aria

  • ARIAButtonAlteredProps
  • ARIAButtonElement

@fluentui/react-positioning

  • usePositioningMouseTarget

@fluentui/react-utilities

  • useIsomorphicLayoutEffect
  • useMergedRefs
  • FluentTriggerComponent
  • TriggerProps

@fluentui/react-shared-context

  • useThemeClassName_unstable
  • useTooltipVisibility_unstable
  • TooltipVisibilityContextValue_unstable
  • ThemeContextValue_unstable

@fluentui/react-context-selector

  • ContextSelector

Detailed API violations per affected package

@fluentui/react-components

import { useIsomorphicLayoutEffect } from '@fluentui/react-utilities';

↓↓↓ Leaks ↓↓↓

export { ❌useIsomorphicLayoutEffect❌ }
import { useMergedRefs } from '@fluentui/react-utilities';

↓↓↓ Leaks ↓↓↓

export { ❌useMergedRefs❌ }
import { useThemeClassName_unstable as useThemeClassName } from '@fluentui/react-shared-contexts';

↓↓↓ Leaks ↓↓↓

export { ❌useThemeClassName❌ }
import { useTooltipVisibility_unstable as useTooltipVisibility } from '@fluentui/react-shared-contexts';

↓↓↓ Leaks ↓↓↓

export { ❌useTooltipVisibility❌ }

@fluentui/react-provider

import type { TooltipVisibilityContextValue_unstable } from '@fluentui/react-shared-contexts';

↓↓↓ Leaks ↓↓↓

export declare type FluentProviderContextValues = Pick<FluentProviderState, 'theme'> & {
    provider: ProviderContextValue_unstable;
    themeClassName: ThemeClassNameContextValue_unstable;
    textDirection: 'ltr' | 'rtl';
    tooltip: ❌ TooltipVisibilityContextValue_unstable; ❌
    
};
import type { ThemeContextValue_unstable } from '@fluentui/react-shared-contexts'; 

↓↓↓ Leaks ↓↓↓

export declare type FluentProviderState = ComponentState<FluentProviderSlots> & Pick<FluentProviderProps, 'targetDocument'> & Required<Pick<FluentProviderProps, 'dir'>> & {
    theme: ❌  ThemeContextValue_unstable ❌ ;
    themeClassName: string;
};

@fluentui/react-aria

/* Excluded from this release type: ARIAButtonAlteredProps */
/** @internal */
type ARIAButtonAlteredProps<Type extends ARIAButtonType> = ...compicated union...


↓↓↓ Leaks ↓↓↓

export declare type ARIAButtonResultProps<Type extends ARIAButtonType, Props> = 
  Props & UnionToIntersection<❌ARIAButtonAlteredProps<Type>❌>;

@fluentui/react-radio

import { ContextSelector } from '@fluentui/react-context-selector';

↓↓↓ Leaks ↓↓↓

export declare const useRadioGroupContext_unstable: <T>(selector: ❌ContextSelector<RadioGroupContextValue, T>❌) => T;

@fluentui/react-accordion

import { ContextSelector } from '@fluentui/react-context-selector';

↓↓↓ Leaks ↓↓↓

export declare const useAccordionContext_unstable: <T>(selector: ❌ContextSelector<AccordionContextValue, T>❌) => T;

@fluentui/react-tooltip

import type { FluentTriggerComponent } from '@fluentui/react-utilities';

↓↓↓ Leaks ↓↓↓

export declare const Tooltip: React_2.FC<TooltipProps> & ❌FluentTriggerComponent❌;
import type { TriggerProps } from '@fluentui/react-utilities';

↓↓↓ Leaks ↓↓↓

export declare type TooltipProps = ComponentProps<TooltipSlots> 
& ❌TriggerProps<TooltipTriggerProps>❌ 
& Pick<PortalProps, 'mountNode'> 
& { ... }

@fluentui/react-popover

import type { ContextSelector } from '@fluentui/react-context-selector';

↓↓↓ Leaks ↓↓↓

export declare const usePopoverContext_unstable: <T>(selector: ❌ContextSelector<PopoverContextValue, T>❌) => T;
import type { FluentTriggerComponent } from '@fluentui/react-utilities';

↓↓↓ Leaks ↓↓↓

export declare const PopoverTrigger: React_2.FC<PopoverTriggerProps> & ❌FluentTriggerComponent❌;
import type { TriggerProps } from '@fluentui/react-utilities';

↓↓↓ Leaks ↓↓↓

export declare type PopoverTriggerProps = ❌TriggerProps<PopoverTriggerChildProps>❌ & {...}
import type { usePositioningMouseTarget } from '@fluentui/react-positioning';

↓↓↓ Leaks ↓↓↓

export declare type PopoverState = {....}  & {
 setContextTarget: ReturnType<❌typeof usePositioningMouseTarget❌>[1];
}

@fluentui/react-dialog

import type { FluentTriggerComponent } from '@fluentui/react-utilities';

↓↓↓ Leaks ↓↓↓

export declare const DialogTrigger: React_2.FC<DialogTriggerProps> & ❌FluentTriggerComponent❌;
import type { TriggerProps } from '@fluentui/react-utilities';

↓↓↓ Leaks ↓↓↓

export declare type DialogTriggerProps = ❌TriggerProps<DialogTriggerChildProps>❌ & { ... }

@fluentui/react-menu

import type { ARIAButtonElement } from '@fluentui/react-aria';

↓↓↓ Leaks ↓↓↓

export declare const useMenuItem_unstable: (props: MenuItemProps, ref: React_2.Ref<❌ARIAButtonElement<'div'>❌>) => MenuItemState;

export declare const useMenuItemCheckbox_unstable: (props: MenuItemCheckboxProps, ref: React_2.Ref<❌ARIAButtonElement<'div'>❌>) => MenuItemCheckboxState;

export declare const useMenuItemRadio_unstable: (props: MenuItemRadioProps, ref: React_2.Ref<❌ARIAButtonElement<'div'>❌>) => MenuItemRadioState;
import type { ContextSelector } from '@fluentui/react-context-selector';

↓↓↓ Leaks ↓↓↓

export declare const useMenuContext_unstable: <T>(selector: ❌ContextSelector❌<MenuContextValue, T>) => T;
export declare const useMenuListContext_unstable: <T>(selector: ❌ContextSelector❌<MenuListContextValue, T>) => T;
import type { FluentTriggerComponent } from '@fluentui/react-utilities';

↓↓↓ Leaks ↓↓↓

export declare const MenuTrigger: React_2.FC<MenuTriggerProps> & ❌FluentTriggerComponent❌;
import type { TriggerProps } from '@fluentui/react-utilities';

↓↓↓ Leaks ↓↓↓

export declare type MenuTriggerProps = ❌TriggerProps❌<MenuTriggerChildProps> & {...}
import { usePositioningMouseTarget } from '@fluentui/react-positioning';

↓↓↓ Leaks ↓↓↓

export declare type MenuState = {....} & 
{
 contextTarget: ReturnType<❌typeof usePositioningMouseTarget❌>[0];
setContextTarget: ReturnType<❌typeof usePositioningMouseTarget❌>[1];
....
}

@fluentui/react-avatar

import { ContextSelector } from '@fluentui/react-context-selector';

↓↓↓ Leaks ↓↓↓

export declare const useAvatarGroupContext_unstable: <T>(selector: ❌ContextSelector❌<AvatarGroupContextValue, T>) => T;

@Hotell Hotell changed the title Hotell/build/api stripping migration feat: enable @internal api stripping in all packages and fix export maps Oct 20, 2022
@fabricteam
Copy link
Collaborator

fabricteam commented Oct 20, 2022

📊 Bundle size report

Unchanged fixtures
Package & Exports Size (minified/GZIP)
priority-overflow
createOverflowManager
3.153 kB
1.299 kB
react-badge
Badge
22.646 kB
7.228 kB
react-badge
CounterBadge
23.536 kB
7.512 kB
react-badge
PresenceBadge
24.096 kB
7.09 kB
react-divider
Divider
16.505 kB
5.925 kB
react-image
Image
10.826 kB
4.286 kB
react-label
Label
9.384 kB
3.883 kB
react-positioning
usePositioning
19.826 kB
7.417 kB
react-progress
Progress
13.526 kB
5.113 kB
react-theme
Single theme token import
69 B
89 B
react-theme
Teams: all themes
29.65 kB
6.444 kB
react-theme
Teams: Light theme
17.486 kB
5.057 kB
react-utilities
SSRProvider
180 B
159 B
🤖 This report was generated against f8bd5808004b220aa15f9dd183c3013c16613426

@size-auditor
Copy link

size-auditor bot commented Oct 20, 2022

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: f8bd5808004b220aa15f9dd183c3013c16613426 (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Oct 20, 2022

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
TreeWith60ListItems.default 138 124 1.11:1
LoaderMinimalPerf.default 272 248 1.1:1
SkeletonMinimalPerf.default 311 292 1.07:1
TextMinimalPerf.default 310 293 1.06:1
BoxMinimalPerf.default 309 294 1.05:1
DropdownManyItemsPerf.default 549 524 1.05:1
HeaderMinimalPerf.default 334 317 1.05:1
ImageMinimalPerf.default 350 332 1.05:1
AvatarMinimalPerf.default 168 161 1.04:1
CarouselMinimalPerf.default 352 337 1.04:1
ReactionMinimalPerf.default 341 327 1.04:1
SegmentMinimalPerf.default 308 296 1.04:1
ChatMinimalPerf.default 645 625 1.03:1
FormMinimalPerf.default 334 323 1.03:1
IconMinimalPerf.default 574 555 1.03:1
ToolbarMinimalPerf.default 802 782 1.03:1
ButtonMinimalPerf.default 135 133 1.02:1
ChatWithPopoverPerf.default 288 283 1.02:1
ListNestedPerf.default 466 457 1.02:1
TableMinimalPerf.default 358 352 1.02:1
AttachmentSlotsPerf.default 875 867 1.01:1
DialogMinimalPerf.default 689 683 1.01:1
DividerMinimalPerf.default 317 314 1.01:1
DropdownMinimalPerf.default 2173 2149 1.01:1
InputMinimalPerf.default 856 844 1.01:1
ItemLayoutMinimalPerf.default 970 965 1.01:1
RosterPerf.default 1713 1692 1.01:1
PopupMinimalPerf.default 554 549 1.01:1
PortalMinimalPerf.default 139 137 1.01:1
ProviderMergeThemesPerf.default 996 983 1.01:1
RadioGroupMinimalPerf.default 392 387 1.01:1
SplitButtonMinimalPerf.default 3301 3276 1.01:1
TableManyItemsPerf.default 1577 1556 1.01:1
VideoMinimalPerf.default 625 616 1.01:1
AttachmentMinimalPerf.default 123 123 1:1
CardMinimalPerf.default 468 469 1:1
DatepickerMinimalPerf.default 4634 4631 1:1
EmbedMinimalPerf.default 2643 2634 1:1
FlexMinimalPerf.default 246 245 1:1
HeaderSlotsPerf.default 674 674 1:1
LabelMinimalPerf.default 333 334 1:1
ListMinimalPerf.default 458 457 1:1
MenuMinimalPerf.default 740 739 1:1
MenuButtonMinimalPerf.default 1357 1352 1:1
RefMinimalPerf.default 187 187 1:1
TooltipMinimalPerf.default 1890 1886 1:1
TreeMinimalPerf.default 700 701 1:1
AccordionMinimalPerf.default 118 119 0.99:1
AnimationMinimalPerf.default 469 473 0.99:1
ButtonOverridesMissPerf.default 1010 1020 0.99:1
CheckboxMinimalPerf.default 1523 1537 0.99:1
SliderMinimalPerf.default 1218 1230 0.99:1
StatusMinimalPerf.default 604 610 0.99:1
TextAreaMinimalPerf.default 403 407 0.99:1
CustomToolbarPrototype.default 2148 2174 0.99:1
ListCommonPerf.default 514 522 0.98:1
ProviderMinimalPerf.default 311 317 0.98:1
ButtonSlotsPerf.default 415 427 0.97:1
ChatDuplicateMessagesPerf.default 204 211 0.97:1
LayoutMinimalPerf.default 309 318 0.97:1
AlertMinimalPerf.default 211 220 0.96:1
ListWith60ListItems.default 485 506 0.96:1
GridMinimalPerf.default 289 303 0.95:1

@fabricteam
Copy link
Collaborator

fabricteam commented Oct 20, 2022

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 1089 1113 5000
Breadcrumb mount 2953 2843 1000
Checkbox mount 2499 2471 5000
CheckboxBase mount 2399 2234 5000
ChoiceGroup mount 4274 4330 5000
ComboBox mount 1243 1251 1000
CommandBar mount 8965 9404 1000
ContextualMenu mount 10870 10102 1000
DefaultButton mount 1312 1397 5000
DetailsRow mount 3569 3479 5000
DetailsRowFast mount 3557 3440 5000
DetailsRowNoStyles mount 3339 3325 5000
Dialog mount 3115 3125 1000
DocumentCardTitle mount 569 533 1000
Dropdown mount 3219 3147 5000
FocusTrapZone mount 1992 1782 5000
FocusZone mount 1980 1769 5000
GroupedList mount 1790 2155 2
GroupedList virtual-rerender 1042 1073 2
GroupedList virtual-rerender-with-unmount 1587 1571 2
GroupedListV2 mount 563 546 2
GroupedListV2 virtual-rerender 521 550 2
GroupedListV2 virtual-rerender-with-unmount 540 548 2
IconButton mount 1905 1800 5000
Label mount 698 720 5000
Layer mount 4079 3921 5000
Link mount 763 763 5000
MenuButton mount 1662 1592 5000
MessageBar mount 2227 2111 5000
Nav mount 3096 3127 1000
OverflowSet mount 1421 1215 5000
Panel mount 2527 2526 1000
Persona mount 1149 1205 1000
Pivot mount 1458 1556 1000
PrimaryButton mount 1489 1469 5000
Rating mount 6463 6462 5000
SearchBox mount 1458 1378 5000
Shimmer mount 2852 2625 5000
Slider mount 2099 1942 5000
SpinButton mount 4522 4499 5000
Spinner mount 705 757 5000
SplitButton mount 2768 2906 5000
Stack mount 754 811 5000
StackWithIntrinsicChildren mount 2236 2446 5000
StackWithTextChildren mount 4379 4533 5000
SwatchColorPicker mount 10037 9943 5000
TagPicker mount 2714 2294 5000
TeachingBubble mount 83049 86087 5000
Text mount 772 775 5000
TextField mount 1392 1451 5000
ThemeProvider mount 1468 1494 5000
ThemeProvider virtual-rerender 966 1024 5000
ThemeProvider virtual-rerender-with-unmount 1984 2055 5000
Toggle mount 1038 1050 5000
buttonNative mount 555 527 5000

@bsunderhus
Copy link
Contributor

bsunderhus commented Oct 27, 2022

Here's a follow up:

@fluentui/react-aria #25403

  • ARIAButtonAlteredProps (✅ Make it external)
  • ARIAButtonElement (✅ Make it external)

@fluentui/react-positioning #25407

  • usePositioningMouseTarget (❓ create new type SetVirtualMouseTarget and make the type external)

@fluentui/react-utilities #25406

  • useIsomorphicLayoutEffect (✅ Make it external, it's exported at react-components)
  • useMergedRefs (✅ Make it external, it's exported at react-components)
  • FluentTriggerComponent (❓ shouldn't be external, inline cast to avoid leaking types)
  • TriggerProps (✅ Make it external)

@fluentui/react-shared-context #25405

  • useThemeClassName_unstable (✅ Make it external, it's exported at react-components)
  • useTooltipVisibility_unstable (✅ Make it external, it's exported at react-components)
  • TooltipVisibilityContextValue_unstable (✅ Make it external)
  • ThemeContextValue_unstable (✅ Make it external, it's part of react-provider surface API)

@fluentui/react-context-selector #25404

  • ContextSelector (✅ Make it external)

@fluentui/react-dialog #25408

  • FluentTriggerComponent (❓ shouldn't be external, inline cast to avoid leaking types)

@fluentui/react-tooltip #25409

  • FluentTriggerComponent (❓ shouldn't be external, inline cast to avoid leaking types)

@fluentui/react-menu #25410

  • FluentTriggerComponent (❓ shouldn't be external, inline cast to avoid leaking types)
  • usePositioningMouseTarget (❓ use SetVirtualMouseTarget)

@fluentui/react-popover #25411

  • FluentTriggerComponent (❓ shouldn't be external, inline cast to avoid leaking types)
  • usePositioningMouseTarget (❓ use SetVirtualMouseTarget)

@Hotell Hotell force-pushed the hotell/build/api-stripping-migration branch 2 times, most recently from 7f99859 to d2f1ed1 Compare November 3, 2022 09:22
Hotell added 24 commits November 8, 2022 15:28
- fix(bundle-size): fix type errors exposed by node 14 typings
- fix(tools): fix type errors exposed by node 14 typings
- fix(typings): fix type errors exposed by node 14 typings
- fix(scripts): fix type errors exposed by node 14 typings
@Hotell Hotell force-pushed the hotell/build/api-stripping-migration branch from dca2cb4 to 2781dbd Compare November 8, 2022 14:29
@Hotell
Copy link
Contributor Author

Hotell commented Nov 8, 2022

This will be replaced by #25575 and followups

@Hotell
Copy link
Contributor Author

Hotell commented Nov 16, 2022

Replaced by #25575

@Hotell Hotell closed this Nov 16, 2022
@Hotell Hotell deleted the hotell/build/api-stripping-migration branch November 16, 2022 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants