Using a standard setup from Create React App, without building or importing anything else, the Fluent Theme import fails on compilation for running the test suite.
npx create-react-app my-app --typescript
cd my-app
npm install @uifabric/fluent-theme
Add Theme.test.tsx
to src
import React from 'react'
import { FluentCustomizations } from '@uifabric/fluent-theme'
describe('Fluent Theme', () => {
it('should pass', () => {
console.log(FluentCustomizations)
expect(true).toEqual(true)
})
})
Run jest Theme
```
FAIL src/Theme.test.tsx
โ Test suite failed to run
C:\Users\ArrayKnight\Git\my-app\node_modules\office-ui-fabric-react\lib\Styling.js:1
({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){import './version';
^^^^^^^^^^^
SyntaxError: Unexpected string
at ScriptTransformer._transformAndBuildScript (node_modules/@jest/transform/build/ScriptTransformer.js:471:17)
at ScriptTransformer.transform (node_modules/@jest/transform/build/ScriptTransformer.js:513:25)
at Object.<anonymous> (node_modules/@uifabric/fluent-theme/lib-commonjs/fluent/src/fluent/styles/Breadcrumb.styles.ts:2:1)
```
Test should pass.
Are you willing to submit a PR to fix? No
Requested priority: High
@Vitalius1 - all of these files that import from office-ui-fabric-react/lib/xyz
will break anyone doing tests. Here's an excerpt from /packages/fluent-theme/src/fluent/styles/Breadcrumb.styles.ts
import { IBreadcrumbStyleProps, IBreadcrumbStyles } from 'office-ui-fabric-react/lib/Breadcrumb';
import { FontWeights } from 'office-ui-fabric-react/lib/Styling';
Can we just change these back to:
import { IBreadcrumbStyleProps, IBreadcrumbStyles, FontWeights } from 'office-ui-fabric-react';
@kenotron the error that is mentioned above looks like is complaining because of import './version';
and the author mentions that it's failing without building it first.
Getting Jest to render node_modules is counter to intent. It would either/both have a substantial negative effect on build time and/or require a special setup to override the default build behavior.
It seems like the references could be updated to point at the lib-commonjs
versions (?) since pointing at root seems to be an issue for the odsp-next
team.
@Vitalius1 - the only reason why it's importing from ./version with that import
keyword is because we weren't using the root import from OUFR.
@Arrayknight - do you know why pointing at root is an issue for odsp-next
? There are a lot of places where we import from root internally...
@kenotron odsp-next
will be choke with a size increase due to the bundler they use... that is from @ThomasMichon explanation he gave me real quick.
Sorry @kenotron I don't. It was just brought up in the PR.
@ThomasMichon & @Vitalius1 - It really seems I do not have any alternative than what the compromise was already checked in. Unless you can come up with something I'm just going to have to soft close this with @ArrayKnight's blessing. Because we have to service multiple teams who package differently, we have to resort to the strange lib-commonjs convention.
@ArrayKnight, I know it's not ideal, but I think all we can do right now is to ask that you do a moduleNameMap with jest config for fluent theme. Until odsp-next
can start using Webpack, we'd be stuck on this scheme for now.
For reference, this is what I added to my jest.config.js
to get the test to pass:
moduleNameMapper: {
...
'^office-ui-fabric-react[/\\\\]lib(.*)': 'office-ui-fabric-react/lib-commonjs$1',
},
If closing the issue makes the most sense go for it. Keeping the issue open might help serve as a reminder and a source for future devs. I'd say this should still be addressed when possible. You may want to document this workaround for now.
It's a bit buried somewhere, but it is documented in our wiki: https://github.com/OfficeDev/office-ui-fabric-react/wiki/Fabric-6-Release-Notes#lib-commonjs
I'll use the "Needs: Author Feedback" to soft-close this issue - Fabric team doesn't close bugs by humans for the sake of friendliness to the community. That label gets removed when there's further comments :) So, this will close with no action on your part. Thanks!
This issue has been automatically marked as stale because it has marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your contributions to Fabric React!
@ArrayKnight this work around unfortunately doesn't work with CRA since they don't support moduleNameMapper. So for right now, we are avoiding using @uifabric/fluent-theme. Is there maybe another work around for this @kenotron? Let me know if there's any more info I can provide
@lalicari I had already ejected on my CRA, so it wasn't an issue. But that does make sense. Sorry this solution doesn't work for you
Hey assigned owner; just a friendly reminder that this issue needs attention. Do you have any status on this? If you need help, please contact the shield dev to see if they can assist.
Yes, I cannot do much more here unless we come up with a different scheme for how we do ESM & commonjs.
Please could you let me know if we have gotten any further with this?
Out team is struggling to test any react components that use @uifabric/fluent-theme inside of them.
I've tried this:
moduleNameMapper: {
'^office-ui-fabric-react[/\\\\]lib(.*)': 'office-ui-fabric-react/lib-commonjs$1'
}
Jest still errors out with the identical error listed in the first post.
We are using react-scripts which uses jest and we are using Typescript.
Any advice please?
@dirkesquire now that 7.x is long out, have you been able to migrate and discontinue using fluent-theme?
This issue has been automatically marked as stale because it has marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your contributions to Fabric React!
Thank you - yes I can confirm I am using 7.x now and I managed to work around the problem, although its a while back now and I can't quite remember how. It might have been just getting the jest.config.js just right. We are still using fluent-theme (we haven't discontinued using it).
This issue has been automatically marked as stale because it has marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your contributions to Fabric React!
Most helpful comment
For reference, this is what I added to my
jest.config.js
to get the test to pass:If closing the issue makes the most sense go for it. Keeping the issue open might help serve as a reminder and a source for future devs. I'd say this should still be addressed when possible. You may want to document this workaround for now.