Fluentui: @fabricui/fluent-theme SSR issues

[bkniffler]

Using fluent-theme will throw errors on using something like nextJS. I know I can import from lib-commonjs, but even those commonjs packages will import stuff like office-ui-fabric-react/lib/Styling which in turn uses es modules (instead of referencing office-ui-fabric-react/lib-commonjs/Styling).

There is more problems like document undefined due to @microsoft/load-themed-styles/lib/index registerStyles not checking if document is defined.

I could go and create the stack traces and detailed error messages, but I guess that the new fluent-theme is still in preview and this has not yet been optimized.

[bkniffler]

https://github.com/Microsoft/web-build-tools/issues/992

[khmakoto]

CC'ing @Vitalius1 as he may have more context given that he is one of the people working on integrating fluent theming.

[Vitalius1]

I am guessing @kenotron or @cliffkoh could be more helpful here as this is more related to imports (not my area of expertise 🙄)

[cliffkoh]

@khmakoto please sync with @kenotron or myself offline to discuss a fix for this.

[khmakoto]

Sorry @Vitalius1, I'll sync with @cliffkoh of @kenotron to discuss this. Thanks.

[kenotron]

Two issues here:

1. SSR support for load-themed-styles is through a configuration - you can consult that package's configure load theme function: https://github.com/Microsoft/web-build-tools/blob/33d670bf12fb70a93b28c55d9103550eeadf0738/libraries/load-themed-styles/src/index.ts#L193

Also, check out the apps/ssr-tests to see how that works

1. Official answer: it's a bug, Fabric needs to fix it so that internally we don't reference anything through lib. Unofficial answer: to support esm in node, you can actually use a package like esm: https://www.npmjs.com/package/esm

[dzearing]

@kenotron I don't think this is a bug here in paths.

Referencing package based imports in the prod code creates massive bloat for AMD bundling partners or any partner using Webpack < 4. In small packages where we know the entire package will be used, this is not a big deal, but for large packages like office-ui-fabric-react, non Webpack-4 bundlers end up pulling everything in.

Instead of changing the path based imports, can partners in this scenario simply map the /lib/ folders to /lib-commonjs/ if they need to resolve commonjs paths? This is what we do for unit tests which still require commonjs.

[kenotron]

I still think there MAYBE be a bug depending on where the imports are.

Looking @khmakoto 's massive conversion PR (#7390), we are mostly dealing with experimental packages and app packages. Scoping wise, this issue is with fluent-theme imports of office-ui-fabric-react/lib/Styling. We should focus on that package in the PR probably.

As @dzearing said, if the imported target is large like OUFR itself, we shouldn't import that whole package. I agree with this. However, /lib/Styling from OUFR is a direct forward of @uifabric/styling. In that case, I would have expected the replacement here in Mak’s PR to be:

import {Foo, Bar} from ‘@uifabric/styling’;

Rather than

import {Foo, Bar} from ‘office-ui-fabric-react’;

[msft-github-bot]

:tada:This issue was addressed in #7390, which has now been successfully released as @uifabric/[email protected].:tada:

Handy links:

• Release Notes
• Official NPM Release