Fluentui: Fabric common-js files import es module files

Created on 23 Oct 2019  路  12Comments  路  Source: microsoft/fluentui

Environment Information

  • Package version(s): office-ui-fabric-react 7.54.1
  • Browser and OS versions: Not relevant.

Please provide a reproduction of the bug in a codepen:

Not a codepen repro, but see this repo to demo this issue.

Actual behavior:

common-js files reference modules which causes Jest and Node to fail since they don't support modules. One fix is to run the fabric through babel, but this isn't an ideal fix.

Our tests were working until the latest update. In our case the issue comes from SuggestionsControl. I haven't tested it, but I think importing from the root should fix the issue.

Expected behavior:

Module files should import modules and common-js should import common-js.

Priorities and help requested:

Are you willing to submit a PR to fix? No

Requested priority: Normal

Packaging Type

Most helpful comment

Thanks @jdhuntington. This configuration works.

A note for Create-React-App user: CRA only supports reading the jest config from the package.json file, and adding the moduleNameWrapper alone will do the trick

``` json
...
"jest": {
"moduleNameMapper": {
"office-ui-fabric-react/lib/(.*)$": "office-ui-fabric-react/lib-commonjs/$1"
}
}
````

All 12 comments

Thanks for the concise repro. Will dig into this this week.

@kenotron did some work in #10561 which I think should have addressed this, but maybe it's not fully working?

@ecraig12345
I can repro this bug with Create React App, importing a component like Button would fail the jest. The bug exists since version 7.54.0, last known good version for me was 7.53.1

Ping

BTW I think this should be a bug because it will block users using Jest or Create-React-App

7.56.0 fixed the issue

no, not fixed

Afraid this still requires a workaround. We plan to fix this out of the box, but haven't been able to prioritize fixing just yet.

For now, using common-js in jest requires modifying the moduleNameMapper directive of jest.config.js. This example from your example repo should do the trick:

//@ts-check

/** @type { import("@jest/types").Config.GlobalConfig } */
module.exports = {
    clearMocks: true,
    moduleFileExtensions: ["ts", "tsx", "js"],
    transform: {
        "^.*\\.tsx?$": "ts-jest"
    },
    testPathIgnorePatterns: ["dist", "/build/", "/node_modules/"],
    globals: {
        "ts-jest": {
            packageJson: "package.json"
        }
    },
    moduleNameMapper: {
    "office-ui-fabric-react/lib/(.*)$": "office-ui-fabric-react/lib-commonjs/$1"
  }
};

Thanks @jdhuntington. This configuration works.

A note for Create-React-App user: CRA only supports reading the jest config from the package.json file, and adding the moduleNameWrapper alone will do the trick

``` json
...
"jest": {
"moduleNameMapper": {
"office-ui-fabric-react/lib/(.*)$": "office-ui-fabric-react/lib-commonjs/$1"
}
}
````

I didn't enable it for fear that it'll break too many things. But maybe good to dig up now.

There is a legit import problem here. I have this addressed in the PR here:

https://github.com/OfficeDev/office-ui-fabric-react/pull/11137

@HarryGifford I pulled your git repo, fixed the import, linked my private oufr build to it, and now your jest example works correctly. Thank you for setting that up!

The build is now published and this is now resolved in the example. Thanks for reporting.

Thanks for the quick fix and the moduleNameMapper workaround is useful for other cases when this happens.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

carruthe picture carruthe  路  3Comments

nekoya picture nekoya  路  3Comments

mattcoxonline picture mattcoxonline  路  3Comments

VincentBailly picture VincentBailly  路  3Comments

carruthe picture carruthe  路  3Comments