Skip to content

Tests fail using sample app documentation #8755

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lancegliser opened this issue Apr 18, 2019 · 11 comments
Closed

Tests fail using sample app documentation #8755

lancegliser opened this issue Apr 18, 2019 · 11 comments

Comments

@lancegliser
Copy link

lancegliser commented Apr 18, 2019

Environment Information

  • Package version(s):
    "office-ui-fabric-react": "^6.167.1",
    "react": "^16.8.6",
    "react-dom": "^16.8.6",
    "react-scripts": "2.1.8"
    "npm": '5.6.0',
    "node": '8.11.1',

  • Browser and OS versions: Not applicable

Reproduction steps

Follow the sample app guide: https://github.com/OfficeDev/office-ui-fabric-react/wiki/Sample-App

Please provide a reproduction of the bug in a codepen:

Testing isn't code penable, at least I haven't learned how yet...

Actual behavior:

$npm test

react-scripts test
FAIL src/App.test.js
● Test suite failed to run

Jest encountered an unexpected token

This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not plain JavaScript.

By default, if Jest sees a Babel config, it will use that to transform your files, ignoring "node_modules".

Here's what you can do:
 • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
 • If you need a custom transformation specify a "transform" option in your config.
 • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

You'll find more details and examples of these config options in the docs:
https://jestjs.io/docs/en/configuration.html

Details:

D:\Projects\create-react-app-fabric-tests\node_modules\office-ui-fabric-react\lib\DocumentCard.js:1
({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){export * from './components/DocumentCard/index';
                                                                                         ^^^^^^

SyntaxError: Unexpected token export

  1 | import React, { Component } from 'react';
> 2 | import {
    | ^
  3 |     DocumentCard,
  4 |     DocumentCardPreview,
  5 |     DocumentCardTitle,

  at ScriptTransformer._transformAndBuildScript (node_modules/jest-runtime/build/script_transformer.js:403:17)
  at Object.<anonymous> (src/App.js:2:1)
  at Object.<anonymous> (src/App.test.js:3:1)

Test Suites: 1 failed, 1 total
Tests: 0 total
Snapshots: 0 total
Time: 2.365s
Ran all test suites related to changed files.

Expected behavior:

The default create-react-app test to pass.

Priorities and help requested:

Are you willing to submit a PR to fix? Yes, if I can be given the guidance required.

This may be as simple as documentation update from someone more familiar. So far though I've spent a lot of time testing various out of date or inaccurate blog solutions.

Not sure if this is a create-react-app issue, a feature blocking you, or totally in this project. I found this PR earlier in create-react-app that would seem to allow the property you desire to be used:
facebook/create-react-app#6633

Referenced documentation:

https://github.com/OfficeDev/office-ui-fabric-react/wiki/Fabric-6-Release-Notes
Note: To resolve commonjs modules in Jest, you can use this config blurb in your jest.config.js:

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

Requested priority: High
Not being able to write even shallow tests using enzyme shallow, react-test-renderer shallow, or reactDom seems to justify a project adoption blocker.

Products/sites affected: None

@ecraig12345
Copy link
Member

Thanks for reporting this! There's a good chance the documentation has gotten out of date. We'll look into it and get back to you in the next couple days.

@lancegliser
Copy link
Author

A note, if it does turn out to be documentation. The card styles no longer correctly lay out the persona. The face gets jammed under the data.

@ecraig12345
Copy link
Member

Sorry about the delay getting back to you on this--we've been super busy preparing for a release and the Microsoft //build conference. As part of that, we actually plan to make some major documentation updates, including updated getting started info and tutorials (which most likely will not use create-react-app). I'll update this issue when we have something to share.

In the meantime, I'd recommend some of the material here (written by some of our team members). It's much more up to date with best practices and includes info about testing.
https://github.com/Microsoft/frontend-bootcamp

@ecraig12345
Copy link
Member

One of my coworkers reminded me that while the frontend-bootcamp material provides a great introduction to Fabric components, styling/theming, and testing, it doesn't provide a realistic environment.

For a more realistic environment, you can try starting from npm init uifabric which @kenotron just wrote. (It's definitely still in beta, so please let us know if you find bugs!) This is probably what we'll be using in the updated getting started documentation.

@lancegliser
Copy link
Author

I appreciate the option. But there's a massive community sitting on existing create-react-app, and likely a continuing group starting that way. Providing an alternate doesn't sound like a solution that avoids alienating the current 66,766 starred followers.

Is there a solution that doesn't involve ejecting?

@kenotron
Copy link
Member

I think there's wisdom in what you say there, @lancegliser, in the popular choice. I merely added a truly one liner solution that's geared towards people who are really into TS and UI Fabric - a solution that is going to be fully supported by the Fabric team itself.

However, CRA example from our docs SHOULD be fixed. So it's more of a bug on the wiki itself. We'll address this.

@kenotron kenotron self-assigned this Apr 25, 2019
@jsdevjournal
Copy link

I think I found the solution to this, please see my answer in Stackoverflow and let me know.

https://stackoverflow.com/a/56592238/1802735

@lancegliser
Copy link
Author

lancegliser commented Jun 21, 2019

@durunvo, I'm hesitant to force my team into the management of all configs as that intends. It's impossible to know what's going to be an issue in the future as it falls out of sync. I'm content to wait on @kenotron for a bit longer.

I took another crack at this today using https://github.com/kenotron/create-react-app-uifabric for guidance. I was able to get everything running and testing for the entire app shell, almost.

An important point that made progress:
All usages of 'office-ui-fabric-react/lib/{component}' will fail to load during tests. Always write just 'office-ui-fabric-react' as your import.

But, I could not fix the last issue. When crafting the shell I use <Customizer {...FluentCustomizations}>. The moment I put that back in, I get this error:

Test suite failed to run

Jest encountered an unexpected token

This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not plain JavaScript.
... 
Details:

{site}\node_modules\office-ui-fabric-react\lib\Styling.js:1
({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){import './version';
                                                                                         ^^^^^^

SyntaxError: Unexpected token import

  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)

I think the docs here are suppose to address this, but the "example here" link is missing: https://github.com/OfficeDev/office-ui-fabric-react/wiki/Fluent-Styles-Moving

@ecraig12345
Copy link
Member

Belatedly coming back to this (sorry)--between the time this was opened and now, I had to deal with a related issue trying to integrate a 3rd-party library into our tests, so now I have a better idea of what's going on.

The root issue is that the files Fabric publishes under lib are ES modules, and Jest still doesn't support ES modules for some reason. Since we also publish CommonJS modules (which Jest understands) under lib-commonjs, typically the workaround is to use a moduleNameMapper config as you mentioned in the first post.

Since you can't currently set the moduleNameMapper config with create-react-app, I'm unfortunately not sure what can be done here besides waiting for facebook/create-react-app#6633 to merge or Jest to add ES module support. (Given that ES modules are now a language-level standard and gaining usage, it seems like Jest will have to add support in the near-ish future, but I have no idea exactly when that will happen.)

One possible workaround is changing your code to import from office-ui-fabric-react/lib-commonjs/*, but that would likely require other configuration (mapping certain things back to use lib) which may not be possible in create-react-app.

A side note, I don't think the "Fluent Styles Moving" page was really ever meant for Fabric consumers, it appears to just be notes from one of our devs about the process of moving Fluent styles from the fluent-theme package into the components themselves. I added a note about that at the top of the page.

@lancegliser
Copy link
Author

Lovely to have such a clear definition of the issue. I suppose we'll just leave this as a marker for the world until another project moves to support it.

Thank you for the work!

@microsoft microsoft locked as resolved and limited conversation to collaborators Sep 26, 2019
@ecraig12345
Copy link
Member

In case anyone finds this later, there appears to be support in create-react-app now for adding a moduleNameMapper config.
facebook/create-react-app#6055

@microsoft microsoft unlocked this conversation Nov 6, 2019
@microsoft microsoft locked as resolved and limited conversation to collaborators Dec 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants