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-popover): removes exposing of internal type FluentTriggerComponent #25411

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-popover 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

@fabricteam
Copy link
Collaborator

fabricteam commented Oct 27, 2022

📊 Bundle size report

Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-alert
Alert
84.688 kB
21.233 kB
react-avatar
Avatar
48.874 kB
13.864 kB
react-avatar
AvatarGroup
14.996 kB
6.013 kB
react-avatar
AvatarGroupItem
63.452 kB
17.959 kB
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-persona
Persona
53.992 kB
15.25 kB
react-popover
Popover
103.342 kB
31.82 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)

@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:20
@bsunderhus bsunderhus requested a review from a team as a code owner October 28, 2022 09:20
@bsunderhus bsunderhus force-pushed the react-popover/fix--removes-exposing-of-internal-type-FluentTriggerComponent branch from 3f9ca1d to e79b408 Compare October 28, 2022 12:10
@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1277 1288 5000
Button mount 925 922 5000
FluentProvider mount 1486 1491 5000
FluentProviderWithTheme mount 574 576 10
FluentProviderWithTheme virtual-rerender 541 533 10
FluentProviderWithTheme virtual-rerender-with-unmount 576 571 10
MakeStyles mount 1950 1982 50000
SpinButton mount 2334 2332 5000

@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 e79b408:

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

@bsunderhus bsunderhus merged commit c5f74a8 into microsoft:master Oct 31, 2022
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