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

Adding tests to CRA example yields an error #159

Closed
akutruff opened this issue Dec 20, 2021 · 6 comments
Closed

Adding tests to CRA example yields an error #159

akutruff opened this issue Dec 20, 2021 · 6 comments

Comments

@akutruff
Copy link

akutruff commented Dec 20, 2021

When trying to setup CRA example for tests, I set package.json to:

  "scripts": {
    "start": "cross-env SKIP_PREFLIGHT_CHECK=true craco start",
    "build": "cross-env SKIP_PREFLIGHT_CHECK=true craco build",
    "test": "cross-env SKIP_PREFLIGHT_CHECK=true craco test"
  },

I also added the jest.config.ts to the directory and added jest dependencies.

Edit: in general, adding tests to a project where the tests and the projects live in the same directory is causing issues. I added a basic TypeScript library with jest tests and those break under GitHub CI.

I get the following failure when trying to run the tests.

node ➜ /workspaces/mono-repo-test/examples/cra (master ✗) $ yarn test
yarn run v1.22.15
$ cross-env SKIP_PREFLIGHT_CHECK=true craco test
The following changes are being made to your tsconfig.json file:
  - compilerOptions.paths must not be set (aliased imports are not supported)

● Validation Error:

  Watch plugin jest-watch-typeahead/filename cannot be found. Make sure the watchPlugins configuration option points to an existing node module.

  Configuration Documentation:
  https://jestjs.io/docs/configuration.html

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
@NiGhTTraX
Copy link
Owner

Hey @akutruff, this looks like facebook/create-react-app#11043.

I also added the jest.config.ts to the directory and added jest dependencies.

With CRA you shouldn't have to manually add jestreact-scripts test (or in this case craco test) should come with everything you need.

FWIW, everything is working fine for me locally. Maybe run yarn why jest-watch-typeahead to see where the dependency the error complains about is located in the monorepo. This is what I get:

yarn why jest-watch-typeahead       
yarn why v1.22.0
[1/4] Why do we have the module "jest-watch-typeahead"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "jest-watch-typeahead@0.6.1"
info Reasons this module exists
   - "_project_#@nighttrax#cra-example#react-scripts" depends on it
   - Hoisted from "_project_#@nighttrax#cra-example#react-scripts#jest-watch-typeahead"

@akutruff
Copy link
Author

Thanks for looking into this and the repo.

package.json has yarn run build for the test script which doesn't execute tests, it just builds it right?

  "scripts": {
    "start": "cross-env SKIP_PREFLIGHT_CHECK=true craco start",
    "build": "cross-env SKIP_PREFLIGHT_CHECK=true craco build",
    "test": "yarn run build"
  },

@NiGhTTraX
Copy link
Owner

@akutruff yes, all of the test scripts here just build the respective package because that's the main objective of this repo - building packages in a monorepo. The jest example covers running actual tests in a monorepo.


P.S.: the script name is test because it's run in CI and building is what I'm testing for. I should change this because it is confusing...

@akutruff
Copy link
Author

akutruff commented Dec 21, 2021

#160

I tried to take this as far as I could, but I still can't get the Button component to resolve. The jest module resolver customization isn't making sense to me unfortunately.

@NiGhTTraX
Copy link
Owner

@akutruff I left some comments in #160, it looks like craco has some quirks when it comes to jest.config.

NiGhTTraX added a commit that referenced this issue Dec 28, 2021
Fixes #159.

It seems like package managers can break the `watchPlugins` resolution
in jest config by deciding to hoist or not to hoist some of
react-script's dependencies (I believe `jest-config` in combination with
`jest-watch-typeahead` is the most likely combo to cause the issue).

`jest-config` (which does the `require.resolve`) gets hoisted to the top:

```console
❯ yarn why jest-config
=> Found "jest-config@26.6.3"
info Has been hoisted to "jest-config"
info Reasons this module exists
   - "workspace-aggregator-2a09db49-5090-4a4d-b8f3-5689124d9bcf" depends on it
   - Hoisted from "_project_#@jest#core#jest-config"
   - Hoisted from "_project_#jest-runner#jest-config"
   - Hoisted from "_project_#jest-runtime#jest-config"
   - Hoisted from "_project_#@NiGhTTraX#cra-example#react-scripts#jest#jest-cli#jest-config"
```

The watch plugin gets nested under `react-scripts`:

```console
❯ yarn why jest-watch-typeahead
=> Found "jest-watch-typeahead@0.6.1"
info Reasons this module exists
   - "_project_#@NiGhTTraX#cra-example#react-scripts" depends on it
   - Hoisted from "_project_#@NiGhTTraX#cra-example#react-scripts#jest-watch-typeahead"
```

When the config is validated the plugin won't be found since it's not on
the same level as `jest-config`. Different dependency graphs, and even
different `yarn` runs can produce different trees, so this is one
tricky issue to debug.

Adding `jest-watch-typeahead` as a direct dependency in the CRA package
_CAN_ cause it to be hoisted to the same level as `jest-config` (adding
it in the monorepo root is probably safer):

```console
❯ yarn why jest-watch-typeahead
=> Found "jest-watch-typeahead@0.6.5"
info Reasons this module exists
   - "_project_#@NiGhTTraX#cra-example" depends on it
   - Hoisted from "_project_#@NiGhTTraX#cra-example#jest-watch-typeahead"
```

It looks like `jest@27.0.4` [attempts to fix this](https://github.com/facebook/jest/pull/11493/files),
though I'm not convinced it actually addresses the root issue, or just
produces a different dependency tree by virtue of upgrading versions.

----

TL;DR: adding `jest-watch-typehead` in the CRA package or in the
monorepo root _CAN_ solve the issue, though it depends on what other
dependencies are present in the monorepo and where the package manager
decides to place them.

----

Rant: `require()`-ing dependencies by strings from configs will never
work the same for everyone. Hopefully tools (especially `jest`) will
migrate to requiring the consumer to import them directly (this is
impossible right now because jest configs are serialized to JSON).
@NiGhTTraX
Copy link
Owner

For anyone coming here for the jest-watch-typeahead problem, see d931391.

NiGhTTraX added a commit that referenced this issue Jan 24, 2022
This reverts the workarounds for #159, #74 and #60, as they are no
longer needed.

There are a few peer dependency warnings, though the examples work just
fine:

- The one for NextJS is tracked in
vercel/next.js#31887.
- CRACO support for CRA@5 is tracked in
dilanx/craco#378.
- The warnings for CRA are tracked in
facebook/create-react-app#11982, except for
the `ts-jest` one which is expected, since we're using CRA's `jest`.
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

2 participants