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

Remove public API usage of ComponentSlotStyle and redefine type internally #2996

Merged

Conversation

JamesBurnside
Copy link
Member

@JamesBurnside JamesBurnside commented Apr 28, 2023

What

Redefine ComponentSlotStyle with interoperable type from fluentui

This works because IRawStyle allows any Record<string, any> but has defined types for common CSS property (of which ComponentSlotStyle also supports) -- so is just a better version of Record<string, any> for styles properties.

Why

We are removing usage of fluent n-star (see #2988) to support react 18 (see #1900) and we need to not rely on this type.

How Tested

Checked for interopability:

image
image

@github-actions
Copy link
Contributor

github-actions bot commented Apr 28, 2023

CallWithChat bundle size is not changed.

  • Current size: 10191714
  • Base size: 10191714
  • Diff size: 0

@github-actions
Copy link
Contributor

github-actions bot commented Apr 28, 2023

Chat bundle size is not changed.

  • Current size: 9839746
  • Base size: 9839746
  • Diff size: 0

@github-actions
Copy link
Contributor

github-actions bot commented Apr 28, 2023

Calling bundle size is not changed.

  • Current size: 9797691
  • Base size: 9797691
  • Diff size: 0

@github-actions
Copy link
Contributor

@github-actions
Copy link
Contributor

Copy link
Member

@emlynmac emlynmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great step forward to react 9!

@github-actions
Copy link
Contributor


import { IRawStyle } from '@fluentui/react';

/**
Copy link
Member

@PorterNan PorterNan Apr 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would that be better to put this one in northstar wrapper?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did consider that but I'm actually against that as we shouldn't ever have publicly exported a fluent northstar interface and should have defined our own interface/used regular fluents in this package.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(It also makes the bundling more difficult if we have a public API coming out of the northstar wrapper the way we rollup the api)

@JamesBurnside JamesBurnside merged commit f3d7805 into main Apr 28, 2023
29 checks passed
@JamesBurnside JamesBurnside deleted the jaburnsi/remove-public-usage-of-componentslotstyle branch April 28, 2023 22:37
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.

None yet

3 participants