Fluentui: Fluent Theme conflicts with Jest

Created on 20 May 2019  ยท  19Comments  ยท  Source: microsoft/fluentui

Environment Information

  • __Package version(s)__:
  • "@uifabric/fluent-theme": "0.16.9"
  • "jest": "24.7.1",
  • "react": "16.8.6"
  • "create-react-app": "3.0.1"
  • "typescript": "3.4.5"

Please provide a reproduction of the bug in a codepen:

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

Actual behavior:

```
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)

```

Expected behavior:

Test should pass.

Priorities and help requested:

Are you willing to submit a PR to fix? No

Requested priority: High

Fluent Author Feedback fluent-theme ASAP By Design No Recent Activity Type

Most helpful comment

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.

All 19 comments

@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!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ravick4u picture ravick4u  ยท  32Comments

lukasbash picture lukasbash  ยท  31Comments

jeremy-coleman picture jeremy-coleman  ยท  63Comments

just-joshing picture just-joshing  ยท  35Comments

JoshuaKGoldberg picture JoshuaKGoldberg  ยท  33Comments