Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Fix the resolve.modules directory order #822

Closed
edmorley opened this issue Apr 27, 2018 · 0 comments · Fixed by #926
Closed

Fix the resolve.modules directory order #822

edmorley opened this issue Apr 27, 2018 · 0 comments · Fixed by #926
Labels

Comments

@edmorley
Copy link
Member

After several hours of debugging I've managed to work out why the Jest 22 upgrade PR was failing when Jest was used with Preact, but not when used with React.

When reproducing locally (inside a project created with create-project), the failure looked like:

test-preact-jest $ yarn test
yarn run v1.6.0
$ neutrino test
 FAIL  test\simple_test.js
  ● Test suite failed to run

    TypeError: Cannot read property 'AsymmetricMatcher' of undefined

      at Object.<anonymous> (../neutrino-dev/node_modules/jest-matcher-utils/build/index.js:31:49)
      at Object.<anonymous> (../neutrino-dev/node_modules/expect/build/index.js:3:25)

The cause was this:

  • jest-matcher-utils (used by jest and present in the yarn linked jest preset directory) uses pretty-format, which gets hoisted to the node_modules in the monorepo root
  • preact-compat (which is in the test project's node_modules) indirectly uses a much older version of pretty-format, which gets hoisted to the root of the test project's node_modules
  • because of the current Neutrino resolve.modules settings, jest-matcher-utils ended up using the older pretty-format from the test project node_modules instead of the monorepo node_modules

Doing a yarn test --debug showed the generated modules list being passed to Jest, as:

  "moduleDirectories": [
    "C:\\Users\\Ed\\src\\neutrino-dev\\packages\\jest\\node_modules",
    "C:\\Users\\Ed\\src\\neutrino-dev\\packages\\eslint\\node_modules",
    "node_modules",
    "C:\\Users\\Ed\\src\\test-preact-jest\\node_modules",
    "C:\\Users\\Ed\\src\\neutrino-dev\\packages\\web\\node_modules",
    "C:\\Users\\Ed\\src\\neutrino-dev\\node_modules",
    "C:\\Users\\Ed\\src\\neutrino-dev\\packages\\preact\\node_modules"
  ],

Doing any of the following stopped the test from failing:

  1. Moving "C:\\Users\\Ed\\src\\neutrino-dev\\node_modules" higher up the list (by making the corresponding monorepo-specific call in the web preset a .prepend())
  2. Removing "C:\\Users\\Ed\\src\\test-preact-jest\\node_modules" (by removing the .add(neutrino.options.node_modules) from the web preset)
  3. Removing all entries other than node_modules (the webpack default). This surprisingly worked for both test & build.

It also seems wrong to not have node_modules as the first entry?

I'll probably end up picking (1) or (2) as a workaround for now (since it will prevent any risk to non-monorepo), however I think we really need to come up with a better order/approach here (within the constraints of having to work with monorepo + standalone + standalong w/no hoisting), since this is not the first time I've spent hours debugging an issue to find it's been due to the Neutrino modules resolution order (another example).

@edmorley edmorley added the bug label Apr 27, 2018
edmorley added a commit that referenced this issue Apr 27, 2018
Notably Jest v22 adds support for babel 7. Other changes:
https://github.com/facebook/jest/blob/master/CHANGELOG.md#jest-2200

The workaround fixes test failures due to `preact-compat`'s older
version of `pretty-format` being used by `jest-matcher-utils` instead
of the version listed in its dependencies. See:
https://travis-ci.org/mozilla-neutrino/neutrino-dev/jobs/371118845#L847
#822

Closes #770.
edmorley added a commit that referenced this issue Jun 4, 2018
Since the custom module resolution is not required, and has been the
cause of at least 3 bugs that I have spent hours debugging.

The vast majority of modules referenced by Neutrino already use
`require.resolve()` and so are unaffected by this change. Those that
do not (for example eslint presets, since eslint doesn't support
specifying them as full paths), were not helped by this custom
module resolution anyway, since the resolution happens outside of
webpack, and so relied on hoisting even before this change.

The `node_modules` option has been removed in favour of using the
Neutrino API (or else `NODE_PATH`). Customising module resolution
is a massive footgun and should not be used in most cases.

Fixes #822.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging a pull request may close this issue.

1 participant