Redwood: `yarn rw test` Error: command doesn't run as of Apollo v3 upgrade

[RodrigoJacznik]

Hi :wave: , I have a problem with a new redwood project.

I run the following commands:

yarn create redwood-app test
cd test
yarn rw g component Test
yarn rw test

And I get the next error:

Jest encountered an unexpected token
...
    /home/rodrigo/workspace/test/node_modules/@apollo/client/react/hooks/useSubscription.js:1
    import { __assign } from "tslib";
    ^^^^^^

    SyntaxError: Cannot use import statement outside a module
        at compileFunction (<anonymous>)

      at Runtime._execModule (../node_modules/jest-runtime/build/index.js:1179:56)

This is the output of yarn rw info

  System:
    OS: Linux 5.8 Arch Linux
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 12.18.3 - /tmp/yarn--1601732607280-0.7441083359073166/node
    Yarn: 1.22.10 - /tmp/yarn--1601732607280-0.7441083359073166/yarn
  Databases:
    SQLite: 3.33.0 - /usr/bin/sqlite3
  Browsers:
    Firefox: 81.0
  npmPackages:
    @redwoodjs/core: ^0.19.1 => 0.19.1 

Let me know if I can help somehow.
Thanks in advance.

[thedavidprice]

Confirming I'm seeing this error on v0.19.1

Full output:

FAIL   web  web/src/components/Test/Test.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:

    /Users/price/Repos/xx-delete/node_modules/@apollo/client/react/hooks/useSubscription.js:1
    import { __assign } from "tslib";
    ^^^^^^

    SyntaxError: Cannot use import statement outside a module
        at compileFunction (<anonymous>)

      at Runtime._execModule (../node_modules/jest-runtime/build/index.js:1179:56)
      at Object.<anonymous> (../node_modules/@redwoodjs/web/dist/index.js:80:24)
      at Object.<anonymous> (../node_modules/@redwoodjs/testing/dist/MockProviders.js:15:12)
      at Object.<anonymous> (../node_modules/@redwoodjs/testing/dist/customRender.js:17:22)
      at Object.<anonymous> (../node_modules/@redwoodjs/testing/dist/index.js:48:21)
      at Object.<anonymous> (../node_modules/@redwoodjs/core/dist/configs/browser/jest.setup.js:16:5)

[thedavidprice]

@RobertBroersma could you take a look at this one? I did a little digging and couldn't find anything simple via updating related @testing-library packages. I also tried upgrading @apollo/client.

My working theory is that @redwoodjs/testing isn't playing nice with changes from upgrading to Apollo v3 in v0.19. Perhaps something with the new exports here:
https://github.com/redwoodjs/redwood/blob/main/packages/web/src/index.ts

[thedavidprice]

@RodrigoJacznik thanks for opening this and appreciate the offer to help! You're welcome to dig in and we'll assist however we can. Jest integration has not been trivial. Here's the link to the code for @redwoodjs/testing package:
https://github.com/redwoodjs/redwood/tree/main/packages/testing

No pressure to dive in. But let us know if you do!

[RobertBroersma]

@thedavidprice I'm not at my PC atm, but does Apollo v3 happen to export untranspiled es6 code?

[thedavidprice]

Looks transpiled.

Couldn't find anything obvious in the current Apollo Issues. Here's the latest (Mocked Provider issue might shed some light for us):
https://github.com/apollographql/apollo-client/issues?q=is%3Aissue+is%3Aopen+jest+

Also briefly looked through testing-library repos for related issues. Nothing obvious.

Reference

In case any of this is helpful:

• v3 Overview Article
• Migration guide: check out the react-testing section
• Hooks specific migration info: testing section

[RobertBroersma]

I'm seeing this error as well, but seems unrelated to apollo.

Details:

    /mnt/c/Home/repos/redwood/packages/structure/dist/model/RWCell.js:3
    import { withCell } from "@redwoodjs/web";
    ^^^^^^

    SyntaxError: Cannot use import statement outside a module
        at compileFunction (<anonymous>)

      33 | var _URL = require("../x/URL");
      34 |
    > 35 | var _RWCell = require("./RWCell");
         |               ^
      36 |
      37 | var _RWComponent = require("./RWComponent");
      38 |

I've seen this before that Jest complains about imports while they are actually properly transpiled... I have no idea why it happens...

[thedavidprice]

I thought @peterp mentioned to me during a conversation this week he could run the tests by removing all the Apollo Client imports. I'll check. And I'm going to experiment with a few things now to see if I can isolate the error. There's also a lot of related packages with upgrades available — possible we're encountering a bug that's been fixed.

[aldonline]

We did some diggin' with @thedavidprice and found the underlying cause. The solution, however, requires making some decisions that affect parts of the codebase that I don't quite understand at this point. I can contribute some clarity and hope that this advances the discussion.

Here's what's going on:

• Apollo v3 introduced a significant change. They are now publishing a CommonJS (CJS) and an ES Modules (ESM) version of the library (they transpile to two targets and ship them to NPM)
  
  
  
  • This is good for tree shaking, for example (webpack can use the ESM version to include only what's necessary)
  
  
  • But it means that there are two completely different set of *.js files shipped with their NPM package
  
  
  • The CJS files (the "traditional" files) end with *.cjs.js. Everything else is ESM.
  
  
  • They are following best practices, and including both a "main" and "module" field sin package.json, pointing to the CJS and ESM entrypoints respectively. So it is not entirely their fault.
  
  

So, if they are following best practices, why do our tests break?
Because we're doing some weird stuff that assumes a few things that are no longer true.
This is hard to explain in the abstract, so I'll just explain it step by step, starting from the error.

This is the error we get:

/private/tmp/twtestissue/testapp/node_modules/@apollo/client/react/hooks/useSubscription.js:1
    import { __assign } from "tslib";
    ^^^^^^

    SyntaxError: Cannot use import statement outside a module
        at compileFunction (<anonymous>)

      at Runtime._execModule (../node_modules/jest-runtime/build/index.js:1179:56)
      at Object.<anonymous> (../node_modules/@redwoodjs/web/dist/index.js:78:24)
      at Object.<anonymous> (../node_modules/@redwoodjs/testing/dist/MockProviders.js:15:12)
      at Object.<anonymous> (../node_modules/@redwoodjs/testing/dist/customRender.js:17:22)
      at Object.<anonymous> (../node_modules/@redwoodjs/testing/dist/index.js:48:21)
      at Object.<anonymous> (../node_modules/@redwoodjs/core/dist/configs/browser/jest.setup.js:16:5)

• This all seems to start with browser/jest.setup.js.
  
  
  
  • To be more specific, at the top of the stack we have "some node process" that eventually runs jest.setup.js. The fact that there is a vanilla Node.js process at the top is important to keep in mind
  
  
• It then loads test/dist/index.js via require()
• A few more requires later, we are now executing web/dist/index.js
• which, in turn, tries to require the following:

var _useSubscription = require("@apollo/client/react/hooks/useSubscription")

• this require call will actually succeed in finding a file to load
• it follows the node resolution algorithm and ends up finding node_modules/@apollo/client/react/hooks/useSubscription.js
• however, this file is an ES Module (not a CommonJS module), so the parser fails when it finds an "import" keyword
  
  
  
  • Keep in mind that this is a vanilla "nodejs process" that is not running with any special flags to process ES Module syntax, or with any registered transpiler (like ts-node - which might be a patch solution to consider). It is just a traditional node process going about its day and loading CJS files.
  
  
  • Also, it doesn't try to read package.json to find the "main" field (which is the CJS entry point) because our require specifier is "deep" (it goes beyond the top level "@apollo/client" and points directly to a folder "@apollo/client/react/hooks/...")
  
  

At this point, we could fix this by changing web/dist/index.js so it requires the CJS version of the file, like so:

var _useSubscription = require("@apollo/client").useSubscription

And this will work (actually, we need to make a few changes to other parts of that file for that variable to be exported correctly, but that's not important at this point).

Of course, we know web/dist/index.js comes from web/src/index.ts, so we should change this in the source file:

// this is how we're doing it today (following Apollo v3 instructions)
// but it compiles down to a require expression that node can't handle

// export { useSubscription } from '@apollo/client/react/hooks/useSubscription'
// export { useLazyQuery } from '@apollo/client/react/hooks/useLazyQuery'
// export { useQuery } from '@apollo/client/react/hooks/useQuery'
// export { useMutation } from '@apollo/client/react/hooks/useMutation'
// export { useApolloClient } from '@apollo/client/react/hooks/useApolloClient'

// so, instead, we could do it this way

export {
  useSubscription,
  useLazyQuery,
  useQuery,
  useMutation,
  useApolloClient,
} from '@apollo/client'

And this will now lead to a transpiled web/dist/index.js code that won't break when the jest process tries to load it.

There might be a few issues with this approach (we need to validate this):

• We might not allow webpack to properly tree-shake the Apollo client when bundling the final redwood app (when webpack is bundling the final app it is using the "compiled" redwood framework in dist, so ESM-modularity is already lost).
• This file (web/dist/index.js) is used in several contexts: Testing, the actual app (and maybe storybook?). I don't know enough to know if modifying it this way will break things

Now, there is one more place where Apollo imports must be changed. And this one is trickier:

web/src/graphql/withCell.tsx

import { Query } from '@apollo/client/react/components/Query'

Following the same logic above, we would need to replace it with something that, once transpiled, allows a Node.js process to correctly resolve it to a CJS file.
For example, something like the following:

import { Query } from '@apollo/client'

Unfortunately, Apollo Client does not export "Query" at the top level.
The closest CJS file in the package is: @apollo/client/react/components/components.cjs.js.

So the final (transpiled) code we need is something like this:

const Query = require("@apollo/client/react/components/components.cjs").Query

Which means that we would need to introduce some sort of try/catch or conditional check to know whether we import from @apollo/client/react/components/components.cjs or from @apollo/client/react/components/Query

Anyway. I can see ways of solving this, but I have some blind spots when it comes to the codebase and the different contexts in which files are loaded/transpiled/bundled, so at this point the best thing I can do is just share my findings.

My best "let's get this fixed quickly" bet right now would be to teach the node process that ends up running Jest to handle ESM syntax when calling require() (this is different than the transpilation process that runs before Jest starts testing). Something like ts-node at the top-level might work.

[aldonline]

This all seems impossibly complex - a labyrinth of gotchas and catch22s.
One of the reasons it is so brittle is because we're trying to compile code (in our dist folders, which are shipped to NPM and end up in node_modules) that is used to achieve multiple goals:

1. It is what the CLI executes
2. Various node processes are require()-ing these files (dev server, language server, jest tests)
3. Webpack is reading these files to create bundles (of the final app)

I think that we might find that treating each of these cases separately can make things easier for us.
So, for example, for the CLI we create a bundle that is optimized to run in Node (and load code lazily as users only require() what they need). For the dev server, we create a bundle that has everything in one file, etc.
For the code that eventually becomes part of the webpack-bundled web app (our user's product), we provide a clean tree of ES Modules, to enable super duper tree shaking.
This is all possible if we start from TypeScript and write custom build scripts.