-
Notifications
You must be signed in to change notification settings - Fork 5
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
Comments
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. |
Can you share a bit more? What does your test look like, how did you configure the workaround? |
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
"transform": {
"^.+\\.(js|ts|tsx)$":
"ts-jest"
},
"transformIgnorePatterns": [
"node_modules/(?!(office-ui-fabric-react/lib/TextField))"
]
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. |
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... |
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. |
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...
|
Just tried the first one. Unfortunately it doesn't work as
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:
|
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:
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:
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 :( |
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.
The problem is, whatever your regex key in 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.
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. |
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. |
So, progress updates here - I'll split this into dealing with ES6 and strings. ES6 ModulesI 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 "^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 "^@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 ModulesI'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 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 |
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( |
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. |
Yes, off course that is a much better solution. |
@surajit09 thank you! |
hey @JakeStanger , I tried the Then I followed the example of your hack and extended it a bit so it ensures all the strings ( for a given You can check out the gist that ensures spfx strings are present while running |
regarding this error, I ended up in updating the
|
I'd probably go with number 2 |
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... |
@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. |
Very very true. Those top level imports seem like the one thing they're sensible enough to leave alone though |
Struggling with this issue right now in the context of testing services using @pnp/* modules. |
@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). |
@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. |
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... |
@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):
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:
Node: 16.8.1 Hours, gone forever. |
Thanks for sharing... this will help cut my time :) BTW, strongly suggest you remove all the |
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. |
@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. |
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.The text was updated successfully, but these errors were encountered: