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

Make icons index.ts/index.d.ts more easier for TS to work with #379

Closed
wants to merge 2 commits into from

Conversation

markjm
Copy link

@markjm markjm commented Jan 12, 2022

Resolves #300, improving TS resolution time for this package.
Also re-casts these functions to unknown first to allow TS to bypass typechecking these icon functions.

@ghost
Copy link

ghost commented Jan 12, 2022

CLA assistant check
All CLA requirements met.

@layershifter
Copy link
Member

layershifter commented Jan 27, 2022

@markjm can you please provide measurements to show before/after? As it's also changing JS output it would be good to see a comparison with Webpack bundling.

For context, we already had an issue with minified output (microsoft/fluentui#20912) that was caused by a bug in acorn (webpack/webpack#14922, acornjs/acorn#1083).

@ling1726
Copy link
Member

@markjm can you please provide measurements to show before/after? As it's also changing JS output it would be good to see a comparison with Webpack bundling.

For context, we already had an issue with minified output (microsoft/fluentui#20912) that was caused by a bug in acorn (webpack/webpack#14922, acornjs/acorn#1083).

#373 resolved the minification issue, but I don't see babelrc being modified in this PR. would be great to check to be sure. Thx!

@layershifter
Copy link
Member

To get measurements with Webpack you need { profile: true, parallelism: 1 } in Webpack config and simply import an icon:

// main.js
import { ICON } from '@fluentui/react-icons'
console.log(ICON)

It will output something like that:

image

@markjm
Copy link
Author

markjm commented Jan 27, 2022

I have recently left Teams, where this was particularly impacting (when TS checking each package seperately). There should be a doc on that floating around somewhere, but I no longer have access to it. I can try to make a dummy app to profile with next week, but I will be away from my computer for a few days.

Aside from being placed in different files, the output javascript should be identical, but the output .d.ts should be more performant.

@tomi-msft tomi-msft closed this Mar 23, 2022
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.

Can @fluentui/react-icons produce a custom .d.ts file which is more performant?
4 participants