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

fix(react-menu): removes exposing of internal type FluentTriggerComponent #25410

Conversation

bsunderhus
Copy link
Contributor

Current Behavior

As described in #25319

We have some internal leaks due to wrongly usage of @internal annotation, the newest API extraction tool will break on those scenarios

New Behavior

This PR solves internal leakages from @fluentui/react-menu by inline casting FluentTriggerComponent usage to avoid leaking internal type, and adopting new SetVirtualMouseTarget type to avoid leaking usePositioningMouseTarget

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

@bsunderhus bsunderhus self-assigned this Oct 27, 2022
@bsunderhus
Copy link
Contributor Author

blocked by #25407

@bsunderhus bsunderhus added the Status: Blocked Resolution blocked by another issue label Oct 27, 2022
@fabricteam
Copy link
Collaborator

fabricteam commented Oct 27, 2022

📊 Bundle size report

Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
62.94 kB
17.663 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
189.603 kB
52.909 kB
react-components
react-components: FluentProvider & webLightTheme
33.446 kB
11.033 kB
react-menu
Menu (including children components)
116.78 kB
36.112 kB
react-menu
Menu (including selectable components)
119.849 kB
36.635 kB
react-portal-compat
PortalCompatProvider
5.857 kB
1.978 kB
🤖 This report was generated against 4d58b78e55691de6f14746dbd282c315771be933

@size-auditor
Copy link

size-auditor bot commented Oct 27, 2022

Asset size changes

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

Baseline commit: ee9ca8f32362b41776a26e4eea182b39f378b6ee (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Oct 27, 2022

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1292 1282 5000
Button mount 939 926 5000
FluentProvider mount 1495 1474 5000
FluentProviderWithTheme mount 568 575 10
FluentProviderWithTheme virtual-rerender 541 535 10
FluentProviderWithTheme virtual-rerender-with-unmount 572 578 10
MakeStyles mount 1972 1977 50000
SpinButton mount 2323 2367 5000

@bsunderhus bsunderhus removed the Status: Blocked Resolution blocked by another issue label Oct 28, 2022
@bsunderhus bsunderhus marked this pull request as ready for review October 28, 2022 09:19
@bsunderhus bsunderhus requested a review from a team as a code owner October 28, 2022 09:19
@bsunderhus bsunderhus closed this Oct 28, 2022
@bsunderhus bsunderhus reopened this Oct 28, 2022
@bsunderhus bsunderhus force-pushed the react-menu/fix--removes-exposing-of-internal-type-FluentTriggerComponent branch from f2868b3 to cd3d6da Compare October 28, 2022 12:10
@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit cd3d6da:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@bsunderhus bsunderhus merged commit 1700a73 into microsoft:master Oct 31, 2022
@bsunderhus bsunderhus deleted the react-menu/fix--removes-exposing-of-internal-type-FluentTriggerComponent branch October 31, 2022 09:04
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Oct 31, 2022
* master: (22 commits)
  fix(react-menu): removes exposing of internal type FluentTriggerComponent (microsoft#25410)
  fix(react-popover): removes exposing of internal type FluentTriggerComponent (microsoft#25411)
  applying package updates
  fix(react-tooltip): removes exposing of internal type FluentTriggerComponent (microsoft#25409)
  chore: Reducing bundle size of Stack by moving selector used in multiple places to local const (microsoft#25429)
  docs(rfcs): Simple component implementation (microsoft#25139)
  Fix migration publishing (microsoft#25422)
  Integrate storywright for story tests - As part of exploring screener alternative (microsoft#25399)
  fix(react-utilities): exposes internal methods used in API surface (microsoft#25406)
  fix(react-dialog): removes exposing of internal type FluentTriggerComponent (microsoft#25408)
  fix(react-context-selector): exposes internal type ContextSelector (microsoft#25404)
  fix(react-aria): exposes internal leaking types (microsoft#25403)
  fix(react-shared-contexts): exposes internal leaks used in the API surface (microsoft#25405)
  fix(react-positioning): exposes new typings to avoid exposing internal methods (microsoft#25407)
  applying package updates
  fix: Allowing DatePicker to be focusable within FocusZones by default (microsoft#25428)
  fix: Pad in slider so the thumb does not render outside the bounds of the root element (microsoft#25378)
  feat: Add enableScopedSelectors prop to Stack that, when true, makes the Stack styles selectors be more scoped to not be as expensive in style recalculation (microsoft#25397)
  fix(react-avatar): Remove gaps between AvatarGroupItem/OveflowButton and its outline (microsoft#25382)
  fix: Combobox text attribute ignored when empty string is passed (microsoft#24665)
  ...
NotWoods pushed a commit to NotWoods/fluentui that referenced this pull request Nov 18, 2022
Hotell pushed a commit to Hotell/fluentui that referenced this pull request Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants