Skip to content
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

Unable to use with ES6 modules #7

Open
JakeStanger opened this issue Sep 27, 2019 · 30 comments
Open

Unable to use with ES6 modules #7

JakeStanger opened this issue Sep 27, 2019 · 30 comments

Comments

@JakeStanger
Copy link
Contributor

JakeStanger commented Sep 27, 2019

As mentioned in #1 I'm getting Unexpected token export. The mentioned workaround does not work for me, as I'm getting this when my React component module imports a component from Office UI Fabric React.

C:\Users\jake.stanger\Development\WebpartLibrary\node_modules\office-ui-fabric-react\lib\TextField.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){export * from './components/TextField/index';
  • SPFx 1.9.1
  • Node 10.16.3
  • Jest 23.6.0
  • @voitanos/jest-preset-spfx-react16 1.2.0
@JakeStanger
Copy link
Contributor Author

Ok, I've managed to get the workaround mentioned in #1 working kinda. It isn't a solution I'm particularly happy with as it dramatically increases the time taken to run tests, and disables automatic type definition generations.

@andrewconnell
Copy link
Member

Can you share a bit more? What does your test look like, how did you configure the workaround?

@JakeStanger
Copy link
Contributor Author

Here is the full test:

import React from "react";
import { NumberEdit } from "./NumberEdit";
import renderer from "react-test-renderer";

test("input changes when number entered", () => {
    const component = renderer.create(
        <NumberEdit valueChanged={console.log} />
    );

    let tree = component.toJSON();
    expect(tree).toMatchSnapshot();
});

I configured the workaround by exactly following the steps outlined in the first post of the aformentioned issue, switching out the exception for transformIgnorePatterns:

  1. Add a transform to the jest config to include JS files:
"transform": {
"^.+\\.(js|ts|tsx)$":
"ts-jest"
},
  1. Add office-ui-fabric-react/lib/TextField to transformIgnorePatterns:
"transformIgnorePatterns": [
"node_modules/(?!(office-ui-fabric-react/lib/TextField))"
]
  1. Add allowJs and remove declaration from tsconfig.json

This succesfully causes the ES6 imports/exports to get transpiled so that Node (and therefore Jest) can run them, but I would really like the to avoid disabling declaration generation on a library.

@andrewconnell
Copy link
Member

This is above as far as I've gotten with this as well. Royal PITA... I've invested a ton of timing trying to track down how to get around this, even to just mock up a bunch of dummy strings, but at this point, I'm at a dead-end. :( Tried posting to multiple React/Jest forums with no luck too...

@JakeStanger
Copy link
Contributor Author

It feels to me like there should be a way of injecting Jest into the build process itself, so that everything exists. Unfortunately I'm not really sure how and it seems like pretty unknown territory.

@andrewconnell
Copy link
Member

andrewconnell commented Oct 2, 2019

Your comment in the sp-dev-docs repo with your hacky workaround got me thinking... I bet there's a way with Jest to tell it to load or map specific files. Maybe something using the moduleNameMapper property...

What if...

  • maybe try using the identity-object-proxy package mapped to the localized string object? that might not work though...
  • maybe point to a new package that could dynamically return a name-value pair object of strings... either one that could return the REAL SPFx strings used in a project, or one that always returned generic dummy strings back

@JakeStanger
Copy link
Contributor Author

Just tried the first one. Unfortunately it doesn't work as moduleNameMapper requires the modules to exist....

Could not locate module AdmWebpartLibraryStrings mapped as:
    identify-obj-proxy.

I think that unfortunately means number 2 isn't going to work either. I don't think Jest by itself has the capability of "creating" modules in the same way webpack does.

Perhaps, though, a tool like Webpack could be configured to create some special test "bundle" where the modules exist, without any of the minification or transformation etc?


With my hackaround in place, and with a mock for the Fabric UI component being used:

jest.mock("office-ui-fabric-react/lib/TextField", () => "TextField");

I'm getting closer to a succesful test, but I'm now having some strange problem where my component import is possibly getting mangled or something.

I know for sure the import works because I use it elsewhere. I've tried changing between a named import, default import and whole module import with no luck. The weird thing is it looks to be imported fine right up until React goes to create it.

import * as React from "react";
import { mount, configure, ReactWrapper } from 'enzyme';
import * as Adapter from 'enzyme-adapter-react-16';

configure({ adapter: new Adapter() });

import NumberEdit, { INumberEditProps, INumberEditState } from "./NumberEdit";

console.log(NumberEdit); // `[Function: NumberEdit]` (and logging a new instance of the class works as expected)

jest.mock("office-ui-fabric-react/lib/TextField", () => "TextField");

describe("NumberEdit", () => {
  let reactComponent: ReactWrapper<INumberEditProps, INumberEditState>;

  beforeEach(() => {
    // This errors, see below:
    reactComponent = mount(React.createElement(
      NumberEdit,
      {
        valueChanged: console.log
      }
    ));
  });

  afterEach(() => {
    reactComponent.unmount();
  });

  it("should change input when number entered", () => {
    reactComponent.simulate("input", "1");
    expect(reactComponent.state("value")).toBe("");
  });
});

Error:

console.error node_modules/react/cjs/react.development.js:228
      Warning: React.createElement: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.

@andrewconnell
Copy link
Member

First, glad to see someone who's got their brain in this space is helping... I wasn't making much progress with it on my own...

@JakeStanger said:

I don't think Jest by itself has the capability of "creating" modules in the same way webpack does.

That's not what I was trying to imply with that comment... rather that property tells Jest how to deal with stuff when it encounters it. IE: "when you hit this JSON file, it's really coming from here"

@JakeStanger said:

Perhaps, though, a tool like Webpack could be configured to create some special test "bundle" where the modules exist, without any of the minification or transformation etc?

I think that's going to be a LOT more trouble than it's worth, and greatly slow test runs down.

I'll try to set aside some time to dig into this more... swamped ATM :(

@JakeStanger
Copy link
Contributor Author

I'd like to get to the bottom of this. I'm not exactly clued up on Jest at all, but I've got enough projects to play with and a little time to spend playing with them.

That's not what I was trying to imply with that comment... rather that property tells Jest how to deal with stuff when it encounters it. IE: "when you hit this JSON file, it's really coming from here"

The problem is, whatever your regex key in moduleNameMapper is, when Jest finds a match, it tries to find the original "module" (anything requireJS will resolve) for it. Since the strings modules aren't there, Jest throws up an error that it can't locate the module because it effectively isn't there.

As far as I can work out, Jest is only capable of mocking/mapping things that definitely do exist, and in this situation we need to map something that doesn't. I presume Jest needs the real thing internally or something.

think that's going to be a LOT more trouble than it's worth, and greatly slow test runs down.

Tests are already being transformed from TS(X) to JS, so I don't think a little extra work on top of that to shove in a few extra tiny modules would slow things down too much. I'd agree Webpack itself is definitely overkill, but something during this process may work.

@JakeStanger
Copy link
Contributor Author

JakeStanger commented Oct 11, 2019

I've not progressed with strings beyond my hackaround unfortunately, however I've managed to succesfully mock a module to test a component using a Fabric UI component.

import "jest";
import * as React from "react";
import { mount } from "enzyme";
import { NumberEdit } from "./NumberEdit";
import { ITextFieldProps } from "office-ui-fabric-react/lib/TextField";

jest.mock("office-ui-fabric-react/lib/TextField", () => ({
  TextField: (props: ITextFieldProps) => (
    <input
      value={props.value}
      onChange={ev => props.onChange(ev, ev.target.value)}
    />
  )
}));

jest.mock("@microsoft/sp-core-library", () => {
  return {
    Environment: "Test",
    EnvironmentType: { Test: "Test" }
  };
});

test("Can mount React component", () => {
  let value = "";

  const component = mount(<NumberEdit onChange={v => (value = v!)} />);
  expect(component).toBeTruthy();

  component.find("input").simulate("change", { target: { value: "1" } });

  expect(value).toBe("1"); //Passes! :)
});

I'm not sure whether this is the better route to go. It adds a lot of development overhead as I'd have to write mocks for every single component I use. The upside is it doesn't require transpiling office-ui-fabric-react, which really increases the testing time.

@JakeStanger
Copy link
Contributor Author

So, progress updates here - I'll split this into dealing with ES6 and strings.

ES6 Modules

I still haven't found a proper catch-all solution, but I have two separate workaround solutions that should at least help deal with a number of packages.

The first, which applies to office-ui-fabric-react, relies on the package shipping with multiple module types. Turns out the Fabric UI package mirrors all of its lib exports with lib-commonjs. This means you can add the following to moduleNameMapper:

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

The second relies on the package being written in TS and shipping its source (a rarity but good to be aware of). We do this with our internal packages. In moduleNameMapper you can switch out both lib and top-level imports to use src. In our case, where all our packages start with @adm/, the following works:

"^@adm/(.*)/lib/(.*)$": "@adm/$1/src/$2",
"^@adm\/([^\/]*)$": "@adm/$1/src",

This also requires the following to whitelist the package and make sure it is actually transformed:

"transformIgnorePatterns": [
    "node_modules/(?!@adm/)"
]

Both of these options are still non-ideal imo but I consider them better than a) mocking anything and b) having to run all JS through TypeScript and having no typings.

Localised Strings Modules

I've basically refined my hack. And made it more of a hack.

It can now pull through the actual string values when the module is created, which is done defining global.define as a method that creates a JS file with an exported strings object. The module is then "required", which causes the custom define function to be called.

Since this runs pre-test it does not affect the testing environment.

const fs = require("fs");
const path = require("path");

const config = JSON.parse(fs.readFileSync("./config/config.json").toString());

const packageJson = stringModule =>
  `{"name":"${stringModule}","main":"index.js"}`;

const rel = pathString => path.join(__dirname, "../", ...pathString.split("/"));

Object.keys(config.localizedResources).forEach(stringModule => {
  if (!fs.existsSync(rel(`node_modules/${stringModule}`))) {
    // set requirejs define function
    global.define = (p1, p2) => {
      fs.writeFileSync(
        rel(`node_modules/${stringModule}/index.js`),
        "module.exports = " + JSON.stringify(p2(), null, 2)
      );
    };

    // try to get strings - check various combinations until the file is found
    let stringsPath = config.localizedResources[stringModule].replace(
      "{locale}",
      "en-us"
    );

    if (!fs.existsSync(rel(stringsPath)))
      stringsPath = stringsPath.replace("lib", "src");

    if (!fs.existsSync(rel(stringsPath)))
      stringsPath = stringsPath.replace("en-us", "en_us");

    if (!fs.existsSync(rel(stringsPath)))
      stringsPath = stringsPath.replace("src", "lib");

    fs.mkdirSync(rel(`node_modules/${stringModule}`));
    require(rel(stringsPath).replace(/\.js$/, ""));
    fs.writeFileSync(
      rel(`node_modules/${stringModule}/package.json`),
      packageJson(stringModule)
    );
  }
});

I still wonder if these can be mocked into some virtual context whereby they don't actually have to be written into node_modules but I'm yet to find anything.

@surajit09
Copy link

I have faced the localised string issue recently. I have bypassed the issue by using a feature in Jest for creating virtual modules (fake module that does not exists yet! Yes you can do that by setting {virtual: true} in the mock) and passing the required data from the virtual module (in my case 'myWPStrings') as:

jest.mock(
'SomeWebPartStrings',
() => {
/*
* Custom implementation of a module that doesn't exist in JS,
* like a generated module or a native module in react-native.
*/
return {someproperty:"some value"};
},
{virtual: true},
);

@JakeStanger
Copy link
Contributor Author

Huh, that looks to be exactly what we were looking for initially and somehow missed. This is a much better solution than my node_modules hack.

Of course the real solution to the strings problem is that your components should not be importing them. I've started only importing strings in the top web part class file, and then passing the object to where its needed as either a prop or React context value.

@surajit09
Copy link

Yes, off course that is a much better solution.

@VladyslavGoloshchapov
Copy link

@surajit09 thank you!

@holylander
Copy link

Huh, that looks to be exactly what we were looking for initially and somehow missed. This is a much better solution than my node_modules hack.

Of course the real solution to the strings problem is that your components should not be importing them. I've started only importing strings in the top web part class file, and then passing the object to where its needed as either a prop or React context value.

hey @JakeStanger , I tried the jest mock solution but I didnt like it too much since I would need to be worrying about mocking all the strings etc...

Then I followed the example of your hack and extended it a bit so it ensures all the strings ( for a given language ) are always present.

You can check out the gist that ensures spfx strings are present while running jest test over here

@holylander
Copy link

As mentioned in #1 I'm getting Unexpected token export. The mentioned workaround does not work for me, as I'm getting this when my React component module imports a component from Office UI Fabric React.

C:\Users\jake.stanger\Development\WebpartLibrary\node_modules\office-ui-fabric-react\lib\TextField.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){export * from './components/TextField/index';
* **SPFx** 1.9.1

* **Node** 10.16.3

* **Jest** 23.6.0

* **@voitanos/jest-preset-spfx-react16** 1.2.0

regarding this error, I ended up in updating the jest config so its transformIgnorePatterns looks like this :

"jest": { "transformIgnorePatterns": [ "node_modules/(?!(@microsoft/sp-dialog|@microsoft/office-ui-fabric-react-bundle|@microsoft/sp-diagnostics|@microsoft/sp-core-library|@microsoft/sp-http|@microsoft/sp-page-context|@microsoft/sp-dynamic-data|@pnp/sp|@pnp/common|@pnp/odata|@pnp))" ],

@JakeStanger
Copy link
Contributor Author

JakeStanger commented Nov 20, 2020

office-ui-fabric-react actually offers commonjs imports, so you can just use those. Two approaches:

  1. Switch out every import from eg /lib/TextField to lib-commonjs/TextField in your code
  2. Use moduleNameMapper: "^office-ui-fabric-react/lib/(.*)$": "office-ui-fabric-react/lib-commonjs/$1

I'd probably go with number 2

@andrewconnell
Copy link
Member

Great solution @JakeStanger! I'm trying to poke a hole in it or find a reason why I shouldn't update the preset to include this. Can you see one?

Hoping to upgrade the presets this coming week or two and would love to include this in the update...

@JakeStanger
Copy link
Contributor Author

JakeStanger commented Nov 27, 2020

@andrewconnell I'd say go for it. The exports are designed to be stable across versions, including major versions.

Edit: actually, I'd test for extensions and library components first. I don't know how it behaves when the module isn't present and Fabric isn't installed on those by default.

@andrewconnell
Copy link
Member

Yeah... but it's OUIFR/Fluent... they design for a lot of stuff they break at a whim in the future with no warning ;)

@JakeStanger
Copy link
Contributor Author

Very very true.

Those top level imports seem like the one thing they're sensible enough to leave alone though

@heinrich-ulbricht
Copy link

Struggling with this issue right now in the context of testing services using @pnp/* modules.

@andrewconnell
Copy link
Member

@heinrich-ulbricht I saw your tweet just now...

I haven't spent time working on this since SPFx bumped their React support to v17.* last November with SPFx v1.16.0. It just slipped my mind. Adding it to my backlog to verify/validate the dependency version matrix first (which is part of my SPFx course), then I'll look to see about updating this package (or, as the name implies, likely publishing a new version for React 17).

@heinrich-ulbricht
Copy link

@andrewconnell This would be great as this hopefully makes it easier to get the SPFx/PnP+Jest combination working. It took me a couple of hours and hair-pulling to make progress. Not sure if I'm there, yet.

@andrewconnell
Copy link
Member

Understood... it's a very tedious process for me as well to figure out the mess of the matrix.

However, I can't commit to when it will be done as I've got a busy week before heading out on a family vacation next week...

@heinrich-ulbricht
Copy link

heinrich-ulbricht commented Jun 27, 2023

@andrewconnell No worries, I now know the state of affairs :)

For me it seems to be working now. I wanted to mock the HttpClient from SPFx and that turned out to be tricky.

I got it working now and so far those are the things that seem to be necessary (although I tried so many things that I'm not 100% sure):

  • mocking the window object (for something that is pulled in via @microsoft/sp-http-base)
  • stubbing the @ms/odsp-core-bundle module via jest.mock (also pulled in by @microsoft/sp-http-base)
  • adding a "^.+\\.resx$": "@glen/jest-raw-loader" transform to Jest config so that SPCoreLibraryStrings.resx (pulled in by @microsoft\sp-core-library) can be required
  • adding Babel config "plugins": ["transform-es2015-modules-commonjs", "transform-amd-to-commonjs"] to convert whatever format @pnp/* is bringing to whatever Jest needs
  • making Babel handle the @pnp/* files by adding this transform to the Jest config: "^.+\\.js$": "babel-jest"
  • adding transformIgnorePatterns to Jest config: "node_modules/(?!(@pnp|@pnp/sp|@uifabric|office-ui-fabric-react|@microsoft/sp-core-library|@microsoft/sp-http|@microsoft/sp-http-base|@microsoft/sp-diagnostics|@microsoft/decorators|@microsoft/sp-page-context|@microsoft/sp-dynamic-data)/)" (copied that from one of the 100 other issues)

Don't get me started on getting debugging to work. But in the end the Jest VS Code extension seems to do its job.

And here's the relevant versions:

  "devDependencies": {
    "@babel/preset-env": "^7.22.5",
    "@glen/jest-raw-loader": "^2.0.0",
    "@microsoft/eslint-config-spfx": "1.16.1",
    "@microsoft/eslint-plugin-spfx": "1.16.1",
    "@microsoft/rush-stack-compiler-4.5": "0.2.2",
    "@microsoft/sp-build-web": "1.16.1",
    "@microsoft/sp-module-interfaces": "1.16.1",
    "@rushstack/eslint-config": "2.5.1",
    "@types/jest": "^29.5.2",
    "@types/react": "17.0.45",
    "@types/react-dom": "17.0.17",
    "@types/webpack-env": "~1.15.2",
    "ajv": "^6.12.5",
    "babel-jest": "^29.5.0",
    "babel-plugin-transform-amd-to-commonjs": "^1.6.0",
    "babel-plugin-transform-es2015-modules-commonjs": "^6.26.2",
    "gulp": "4.0.2",
    "identity-obj-proxy": "^3.0.0",
    "jest": "^29.5.0",
    "jest-date-mock": "^1.0.8",
    "jest-localstorage-mock": "^2.4.26",
    "jest-mock-extended": "^3.0.4",
    "spfx-fast-serve-helpers": "~1.16.3",
    "ts-jest": "^29.1.0",
    "typescript": "4.5.5"
  }

Node: 16.8.1

Hours, gone forever.

@andrewconnell
Copy link
Member

Thanks for sharing... this will help cut my time :)

BTW, strongly suggest you remove all the ^ notations in your versions for applicable test-related packages. Only take dependencies on specific versions (do this by including -DE when you add packages using NPM.

@andrewconnell
Copy link
Member

BTW, I do my best to avoid adding anything to the OOTB configuration that stubs/mocks Microsoft/PnP-specific packages. In my experience, that's a bit of a hornet's nest for each person's project & my preset's goal is to hit the lowest common denominator.

IOW, someone who's not using the PnP React controls shouldn't have stuff in their project that addresses it for testing due to unforeseen side-effects. But if someone else is using those controls, they can install the present & then add those extra configurations to their project.

@heinrich-ulbricht
Copy link

heinrich-ulbricht commented Jun 27, 2023

@andrewconnell This does not make the task easier ^^ No hard feelings if all we end up with is a large wiki page describing everything one could try to get around things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants