cc @dzearing

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 4270bdc:

📊 Bundle size report

_{🤖 This report was generated against e6fa9864d66e5cdf4e965b6069f28c4455dd35dc}

Asset size changes

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

Baseline commit: e6fa9864d66e5cdf4e965b6069f28c4455dd35dc (build)

Perf Analysis (@fluentui/react)

No significant results to display.

thanks for your contribution !

To provide some additional context:

• we have this rule setup but only for external re-exports @rnx-kit/no-export-all, as that's the only issue we identified to mitigate duplicates and tree shake-ability during bundling. Can you pls provide repro that showcases/proves your statement ?
  • also you're referring to import starts (import *) but you're changing re-exports
• not sure you are allowed to use type in v8 packages because old TS support ( cc @ecraig12345 )

• we have this rule setup but only for external re-exports @rnx-kit/no-export-all, as that's the only issue we identified to mitigate duplicates and tree shake-ability during bundling. Can you pls provide repro that showcases/proves your statement ?

Sorry, I have a repro in an internal repository. If you hit me up on Teams, I can provide you more context.

• also you're referring to import starts (import *) but you're changing re-exports

Sorry, that's a typo. It should be fixed now.

• not sure you are allowed to use type in v8 packages because old TS support ( cc @ecraig12345 )

There are currently 2045 instances of import type in the repo. 😄

@Hotell The type comment - I added them a while ago as they've been in typescript since 3.8. Inline type imports though are new, but I don't see them here.

Pending @Hotell approval here. :)

@Hotell: I regret not figuring out a way to remove all export stars tbh. Even though it's more work to maintain, it would also help in resolving even more edge cases. I found this one:

a.js:

export { createTheme } from 'libA';

b.js:

export { createTheme } from 'libB';

index.js:

export * from './a';
export * from './b';

In this case, createTheme is still coming from 2 separate libraries (even though we banned export * from external libraries, it's transitively exported here.) I don't believe typescript will ever complain about this scenario, even if they resolve to separate impls. But let's say both versions map to the same impl within the repo. Then outside of the repo due to node module resolution, you can STILL resolve to different versions, leading to an ambiguous or breaking situation.

Banning all export * would remove this edge case because the index.js file would need to explicitly choose which createTheme to take as the source of truth.

Tommy suggested having a mode in the lint rule to enforce them in just index files. That would help this scenario as well.

• not sure you are allowed to use type in v8 packages because old TS support

import type is fine now since our minbar for v8 is 3.9 (it was only a problem in v7 where our minbar was 3.5).

One random interesting thing about this PR: having the API report file made it really easy to verify that the actual exports stayed the same.

thanks for additional context @dzearing !

based on your new findings, IMO this should be only needed for barrel files that are part of public API -> thus src/index.ts, correct ?

Q: not sure if that lint rule supports such a feature or we need to add additional churn via overrides.
A: it doesn't support. we'll need to use overrides. Created PR for all v9 https://github.com/microsoft/fluentui/pull/22073/files

Last but not least this should be applied across whole monorepo. I'll follow up after we resolve that PR I linked

@tido64

Sorry, I have a repro in an internal repository. If you hit me up on Teams, I can provide you more context.

for future I'd suggest to leverage stackblitz for any kind of isolated repros so anyone is able to troubleshoot.

merged, thx for your contribution @tido64 !