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

Jest Test in Create-React-App broken #215

Open
Firionus opened this issue Dec 6, 2020 · 17 comments
Open

Jest Test in Create-React-App broken #215

Firionus opened this issue Dec 6, 2020 · 17 comments
Labels
help wanted Extra attention is needed todo

Comments

@Firionus
Copy link

Firionus commented Dec 6, 2020

Title

  • bug: Jest Test in Create-React-App broken with TypeError: tabulator_tables_1.default is not a constructor

Environment Details

  • react-tabulator version: 0.14.2
  • OS: Windows 10
  • Node.js version: 14.5.1 (latest LTS)

Long Description
To reproduce:

Result:

(node:9100) UnhandledPromiseRejectionWarning: TypeError: tabulator_tables_1.default is not a constructor
(node:9100) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)
(node:9100) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
 PASS  src/App.test.tsx
  √ renders learn react link (45 ms)

Expected Result: No Warnings

Test can even be made to fail by making it async:

test('renders learn react link', async () => {
  render(<App />);
  const linkElement = screen.getByText(/learn react/i);
  expect(linkElement).toBeInTheDocument();
  await waitFor(() => {
    expect(screen.queryByText("nowhere to be found")).not.toBeInTheDocument();
  });
});

yielding the test result:

 FAIL  src/App.test.tsx
  × renders learn react link (46 ms)

  ● renders learn react link

    TypeError: tabulator_tables_1.default is not a constructor

      at default_1.<anonymous> (node_modules/react-tabulator/lib/ReactTabulator.js:111:25)
      at step (node_modules/react-tabulator/lib/ReactTabulator.js:57:23)
      at Object.next (node_modules/react-tabulator/lib/ReactTabulator.js:38:53)
      at fulfilled (node_modules/react-tabulator/lib/ReactTabulator.js:29:58)

Unfortunately, the test works in Codesandbox (https://codesandbox.io/s/vr3in).

Workaround

No idea right now.

Related Issue
#204

I tried npm cache clean --force, followed by deleting node_modules and npm install, but npm test still gives the same FAIL.

@ngduc
Copy link
Owner

ngduc commented Jan 23, 2021

unit tests work find for commits in master and local, see "yarn test" in:
https://github.com/ngduc/react-tabulator/blob/master/.github/workflows/node.js.yml

I think this error is from an older version "tabulator_tables_1.default is not a constructor", upgrade react-tabulator to the lastest version should fix it.

@Firionus
Copy link
Author

With the current release, I can still reproduce on another machine:

  • react-tabulator version: 0.14.4
  • tabulator-tables version: 4.9.3
  • OS: Ubuntu 20.04.1 LTS
  • Node.js version: 14.15.3
  • npm version: 6.14.9

CI aside, can't you reproduce on your machine?

@ngduc
Copy link
Owner

ngduc commented Feb 26, 2021

Yes, there is some issue when it runs in local, sometimes it fails to launch puppeteer and got stuck there. You may want to kill node processes and try to run the unit tests again.
Updated here: https://github.com/ngduc/react-tabulator/blob/master/docs/development.md#unit-tests

@ngduc ngduc added todo help wanted Extra attention is needed labels Feb 26, 2021
@mike-lischke
Copy link

Here we are, half a year later and with react-tabulator version 0.15.0, and I still see this issue, while writing Jest tests. Since half of my app depends on (react-)tabulator, I'm pretty much lost when it comes to writing tests for that part.

Is there any progress happening meanwhile? Or is a workaround available? Since I also had trouble with monaco-editor, I tried to apply the same trick, first to tabulator-tables then to react-tabulator:

        "transformIgnorePatterns": [
            "/node_modules/(?!monaco-editor)/",
            "/node_modules/(?!react-tabulator)/"
        ],
        "moduleNameMapper": {
            "^monaco-editor$": "monaco-editor/esm/vs/editor/editor.api",
            "^react-tabulator$": "react-tabulator/lib/index.js",
        }

but that didn't help. Note: react-tabulator works without trouble in the application (many thanks for that, btw.!), just not in Jest tests.

@ngduc
Copy link
Owner

ngduc commented Sep 1, 2021

Sorry about this intermittent puppeteer launching issue in local.
I think upgrading to a newer version of puppeteer may help solve this one.
If you could help upgrade it, it'd be great. I will try to do that also.

@timiscoding
Copy link

timiscoding commented Sep 12, 2021

I'm pretty sure the issue I have is related to this and #204 so rather than write a new bug report, I'll just add my thoughts here.

I've been looking at adding a iOS app to our desktop webapp using the Meteor Cordova framework which essentially runs in a webview served over a local web server and I was seeing an error like Unhandled Promise Rejection: TypeError: undefined is not a constructor (evaluating 'new tabulator_tables_1["default"]') from one of our components that used React-tabulator which was odd seeing as our desktop app using the same code was working fine.

Looking in the network tab in devtools I discovered that the only difference was in the way the tabulator-tables dependency was being loaded. Specifically, desktop safari was loading {"module": "dist/js/tabulator.es2015.js"} while in iOS safari, it was loading {"main": "dist/js/tabulator.js"} in package.json. Apparently, the package author added ESM support from 4.8.0 if you read the other GH issue I linked above.

But why was it loading different modules? To answer that question you have to understand that Meteor builds 2 separate bundles, one for modern web browsers called web.browser and the other for older browser support called 'web.browser.legacy'. When building for mobile aka web.cordova it's the same as the legacy build. So in desktop, it used the newer ESM module but in mobile, it was falling back to CJS.

So I tried forcing Cordova to use the ESM module by cloning the tabulator-tables repo into my project to force Meteor to build it. I updated the main field in package.json to point to tabulator.es2015.js and lo and behold it worked. Ugly workaround but it worked.

Next I looked at why this error was occurring in the first place. Turns out, after much hair pulling that there's a bunch of interoperability issues with ESM and CJS modules. React-tabulator uses ESM and when it tries to import tabulator.js, ie. the CJS module from tabulator-tables, it blows up.

The solution? TypeScript has the esModuleInterop compiler option that smooths out default CJS imports, precisely what React-tabulator is doing:

import Tabulator from 'tabulator-tables';

Tested this out by running the repro in this issue and npm linking to a React-tabulator 0.15.0 rebuild:

// package.json
build: "npm run clean && tsc --esModuleInterop --outDir ./lib --jsx react --declaration ./src/index.ts && npm run postbuild"

@timiscoding
Copy link

I've created a npm fork @timiscoding/react-tabulator if anyone wants to test the fix. But ideally, you'll probably want to use patch-package so you don't have to update all your imports in the code base.

As you can see from the diff, the require('tabulator-tables') import is now applied to __importDefault() which falls back to creating an object with a default property to the module.

timiscoding@0decbd2#diff-e6427b57cc4fd1d3909cd8ff56371e8a4e23b90dc66ab1995abb37d1990c076aR98

timiscoding added a commit to Back2Dev/attendance that referenced this issue Sep 13, 2021
react-tabulator eg. admin/forms/list won't show list items. see
ngduc/react-tabulator#215
@mike-lischke
Copy link

@timiscoding Thanks for working this out! I'd prefer, however, that @ngduc takes this in to the original react-tabulator node module. Would make things easier on my end.

In any case, I'm happy to see there's a solution.

@timiscoding
Copy link

@mike-lischke Of course, the best way would be to update the original package. Hoping to get confirmation that it indeed fixes it for others.

@Lost708
Copy link

Lost708 commented Sep 27, 2021

@timiscoding - I downloaded your fork today to see if it fixed the issues we were having with React unit tests failing. Indeed it did! Would be GREAT if this was incorporated into an update in the original package. Thank you for addressing the issue.

@mike-lischke
Copy link

mike-lischke commented Oct 5, 2021

@timiscoding I also found the time finally to test your fork and also for me it worked. Many thanks!

However, I see a problem with mounting the underlying tabulator table in my componentDidMount and componentDidUpdate() methods (running from Jest tests). The table field of my ref.current is undefined in both cases, as if tabulator was never mounted.

I don't know if that's a specific problem of your fork or something that @ngduc should look at. Is it possible to "force" mounting the underlying tabulator when the ReactTabulator is mounted?

@timiscoding
Copy link

timiscoding commented Oct 5, 2021

@mike-lischke Good to hear it! Are you using react-test-renderer? Refs don’t work because it doesn’t mimic a full dom environment. So you have to mock the refs using ‘create NodeMock’ api https://reactjs.org/docs/test-renderer.html#ideas

facebook/react#7740

The only change I made in the fork was in the build step so apart from that, the code would be functionally identical to 0.15.0

@mike-lischke
Copy link

Thanks for the hint. Yes, I'm using react-test-render but also enzyme, where I can mount components. But right, that doesn't mean they also have a DOM. Looks like I need to see what I can do with createNodeMock or similar.

Doesn't belong to the discussion here, however. I hope your changes will be taken over to the main repo sooner than later. Have a good time!

@timiscoding
Copy link

I haven’t used enzyme in a while but could be related to incompatible adapters?

https://stackoverflow.com/a/55292957

@mike-lischke
Copy link

The current member is set in my tests. What's missing is the current.table, i.e. the underlying Tabulator instance is not yet created.

timiscoding added a commit to Back2Dev/attendance that referenced this issue Nov 9, 2021
* tim-survey-builder: (26 commits)
  add readme
  Question types render own Inspector properties. Update parts atom to include type config.
  Enable importing absolute paths. Restructure folders.
  Recoil devtools. Mobile navbar views. Move items with animation.  Dnd lock axis.
  Improve mobile responsive for frame + single type. Refactor DND to avoid unneeded part rerenders.
  Short answer integration
  Start adding mobile responsiveness. Fix recoil not working in ios. Fix storybook startup issue due to new Framework imports.
  Fix ios bug where any component using react-tabulator eg. admin/forms/list won't show list items. see ngduc/react-tabulator#215
  Add ios build
  Add dragndrop for canvas
  Reinstalling storybook
  Load editor's json into builder. Builder can update editor source on clicking save. Update DND. Update placeholder type.
  Enable builder in editor without blowing up. Fix circular imports.
  Refactor recoil state. Load json and save source. Fix error in engine.js when using strict mode.
  Add toolbar to generate source. Refactor to make it easier to add new question types.
  Update single item styles
  Reduce storybook startup time. Update dnd for single items.
  Fix recoil + rbd bug github/facebookexperimental/Recoil#496 Fix styled-components bug github/styled-components/styled-components#3564
  Merge in Minh's short question code. Add dnd to single items.
  EditProperty can auto render with complex paths Modify single items in canvas and inspector
  ...
@kakarot218
Copy link

@timiscoding is it safe to use your package?

@timiscoding
Copy link

The only change is adding a ts compiler option in the build script and running it. I haven't used the original package since my last comment so I'd try the latest release to see if it has already fixed this issue.

timiscoding@0decbd2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed todo
Projects
None yet
Development

No branches or pull requests

6 participants