IconNames: Deprecate due to const enum usage#8005
IconNames: Deprecate due to const enum usage#8005msft-github-bot merged 9 commits intomicrosoft:masterfrom JasonGore:jg/7110-deprecate-iconNames
Conversation
|
Hello @JasonGore! Because this pull request has the Do note that I've been instructed to only help merge pull-requests of this repository that have been opened for at least 8 hours, a condition that is not currently met. No worries though, I will be back when the time is right! 😉 |
…nGore/office-ui-fabric-react into jg/7110-deprecate-iconNames
|
Size Auditor did not detect a change in bundle size for any component! |
packages/icons/src/IconNames.ts
Outdated
| } | ||
|
|
||
| /** | ||
| * @deprecate IconNames is deprecated. |
There was a problem hiding this comment.
Why is this deprecated? I don't think the type IconNamesInput would need to be deprecated.
Also, everything in the @uifabric/icons package is generated by the icon subsetting tool. You can talk with @Jahnp here, but we should change the tool first if we want to make any changes.
There was a problem hiding this comment.
It is dependent on an deprecated object, and could also result in the issue people are seeing in #7110, so that is why I also marked this deprecated. Does that make sense?
I will talk to Peter about the tool, thanks.
There was a problem hiding this comment.
More simply put maybe I should have just said it's a deprecated type, since it's keying off a deprecated object. 😄
There was a problem hiding this comment.
I believe @ThomasMichon added this. A better approach would be to keep the type, but just generate the type. adding this was a cheap way to get some type safety.
also, the tag should be @deprecated, not @deprecate.
There was a problem hiding this comment.
I removed this change before the PR was merged.
|
Last I checked, SPFx was using string union type, but will double check. cc: @patmill |
|
Spoke with @Jahnp and we may decide to close this PR and make another one from the upstream tool changes. I'll mark "Do not merge" for now and close/update within the next day or so. |
|
@JasonGore and I chatted some more offline. This works great for now--we'll go ahead and deprecate the |
KevinTCoughlin
left a comment
There was a problem hiding this comment.
Spoke with SPFx offline, signing-off
|
🎉 Handy links: |
|
🎉 Handy links: |
@Jahnp, is there a road-map w/ ETA for the 7.0 release? |
|
@BTC-Bradley project planning for 7.0 is early but actively being worked on. We're hoping to have it out within the next few months, but that's pending some investigations. You can track progress here (project is being built up & managed by @jdhuntington): https://github.com/OfficeDev/office-ui-fabric-react/projects/29 |
Pull request checklist
$ npm run changeDescription of changes
Per #7110 deprecate
IconNamesdue to its usage ofconst enum.Microsoft Reviewers: Open in CodeFlow