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.

Sign up for GitHub

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

Jump to bottom

relay-compiler has implicit dependency on lodash>=4 and core-js<3 #2759

Closed
wyattanderson opened this issue Jun 6, 2019 · 8 comments
Closed

relay-compiler has implicit dependency on lodash>=4 and core-js<3 #2759

wyattanderson opened this issue Jun 6, 2019 · 8 comments

Comments

@wyattanderson
Copy link

Hi there! I've been working on upgrading our codebase to Relay 4.0.0 and ran into a few roadblocks. It seems that relay-compiler has an implicit (i.e. not specified in package.json and therefore not resolved by npm install when installing relay-compiler) dependency on lodash>=4, and that if your application explicitly pins lodash<4 (for historical reasons, let's not dive into that), relay-compiler can't run at all because it can't import lodash/clone:

ERROR:
Error writing modules:
Error: Cannot find module 'lodash/clone'
    at Function.Module._resolveFilename (module.js:538:15)
    at Function.Module._load (module.js:468:25)
    at Module.require (module.js:587:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/Users/wyatt/git/relay-lodash-reproducer/node_modules/relay-compiler/bin/relay-compiler:24273:18)
    at __webpack_require__ (/Users/wyatt/git/relay-lodash-reproducer/node_modules/relay-compiler/bin/relay-compiler:30:30)
    at _clone (/Users/wyatt/git/relay-lodash-reproducer/node_modules/relay-compiler/bin/relay-compiler:24226:39)
    at keys.forEach.key (/Users/wyatt/git/relay-lodash-reproducer/node_modules/relay-compiler/bin/relay-compiler:24257:38)
    at Array.forEach (<anonymous>)
    at builder (/Users/wyatt/git/relay-lodash-reproducer/node_modules/relay-compiler/bin/relay-compiler:24253:8)

I've built a minimal reproducer at https://github.com/wyattanderson/relay-lodash-reproducer which you can trigger by cloning, running npm install, and then running node_modules/.bin/relay-compiler --extensions jsx --schema schema.graphql --src src/, which will crash with the above error.

And, there's a bonus! On the core-js branch in that repository, I've pinned core-js@3.1.3 (the latest version), and the same steps reproduce the issue in #2701:

Error: Cannot find module 'core-js/es6'
    at Function.Module._resolveFilename (module.js:538:15)
    at Function.Module._load (module.js:468:25)
    at Module.require (module.js:587:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/Users/wyatt/git/relay-lodash-reproducer/node_modules/relay-compiler/bin/relay-compiler:15037:18)
    at __webpack_require__ (/Users/wyatt/git/relay-lodash-reproducer/node_modules/relay-compiler/bin/relay-compiler:30:30)
    at Object.<anonymous> (/Users/wyatt/git/relay-lodash-reproducer/node_modules/relay-compiler/bin/relay-compiler:9663:1)
    at __webpack_require__ (/Users/wyatt/git/relay-lodash-reproducer/node_modules/relay-compiler/bin/relay-compiler:30:30)
    at Object.<anonymous> (/Users/wyatt/git/relay-lodash-reproducer/node_modules/relay-compiler/bin/relay-compiler:14950:1)
    at __webpack_require__ (/Users/wyatt/git/relay-lodash-reproducer/node_modules/relay-compiler/bin/relay-compiler:30:30)
@wyattanderson wyattanderson changed the title relay-compiler has implicit dependency on lodash>=4 relay-compiler has implicit dependency on lodash>=4 and core-js<3 Jun 6, 2019
@wyattanderson
Copy link
Author

This is still an issue with 5.0.0.

@sibelius
Copy link
Contributor

check here https://github.com/facebook/relay/search?q=lodash%2Fclone&unscoped_q=lodash%2Fclone

it is only a dev dependency on lodash when running

yarn why

sibelius-pro:relay sibelius$ yarn why lodash/clone
yarn why v1.16.0
[1/4] 🤔  Why do we have the module "lodash/clone"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "lodash@4.17.11"
info Reasons this module exists
   - "eslint-plugin-flowtype" depends on it
   - Hoisted from "eslint-plugin-flowtype#lodash"
   - Hoisted from "@babel#core#lodash"
   - Hoisted from "@babel#types#lodash"
   - Hoisted from "@babel#traverse#lodash"
   - Hoisted from "@babel#generator#lodash"
   - Hoisted from "eslint#lodash"
   - Hoisted from "babel-preset-fbjs#@babel#plugin-transform-block-scoping#lodash"
   - Hoisted from "@babel#plugin-transform-modules-commonjs#@babel#helper-module-transforms#lodash"
   - Hoisted from "eslint#table#lodash"
   - Hoisted from "eslint#inquirer#lodash"
   - Hoisted from "babel-preset-fbjs#@babel#plugin-transform-classes#@babel#helper-define-map#lodash"
   - Hoisted from "jest#jest-cli#jest-config#jest-environment-jsdom#jsdom#request-promise-native#request-promise-core#lodash"
info Disk size without dependencies: "4.86MB"
info Disk size with unique dependencies: "4.86MB"
info Disk size with transitive dependencies: "4.86MB"
info Number of shared dependencies: 0
✨  Done in 1.04s.

not sure why is this throwing for you

@robrichard
Copy link
Contributor

might be a symptom of the same issue i described here: #2698 (comment)

@wyattanderson wyattanderson mentioned this issue Jun 12, 2019
Closed
@wyattanderson
Copy link
Author

might be a symptom of the same issue i described here: #2698 (comment)

I think that might be it, at least for me! Thanks! Hoping that #2764 fixes it.

@kassens
Copy link
Member

kassens commented Jun 12, 2019

Something is broken in the webpack config we use.

After the build, the dist/relay-compiler file contains a bunch of require('lodash/.*) that I don't know why they're there. I think those should all be inlined during compilation?

@kassens
Copy link
Member

kassens commented Jun 12, 2019

Just saw #2764, that makes sense to me. Fingers crossed that fixes it!

facebook-github-bot pushed a commit that referenced this issue Jun 12, 2019
@facebook-github-bot
Don't include @babel packages in generated bundles (#2764)
70453f2 
Summary:
See background in #2759.

Before this change, the `relay-compiler` binary pulls in `lodash`:

```
dist/relay-compiler/bin/relay-compiler
23734:module.exports = require("lodash/clone");
26487:module.exports = require("lodash/uniq");
27035:module.exports = require("lodash/isPlainObject");
27041:module.exports = require("lodash/isRegExp");
28413:module.exports = require("lodash/isInteger");
28419:module.exports = require("lodash/repeat");
```

And, only these files in `dist/` reference `lodash`:

```
$ rg lodash dist/ -l
dist/relay-compiler/relay-compiler.js
dist/relay-compiler/relay-compiler.min.js
dist/relay-compiler/bin/relay-compiler
```

After this change, no files in `dist/` contain references to `lodash`. This seems consistent with what's contained in Relay packages before they were upgraded to Babel 7 (where the `@` was added) in #2536.
Pull Request resolved: #2764

Reviewed By: alunyov

Differential Revision: D15793295

Pulled By: kassens

fbshipit-source-id: 14e9cc6d83a57d6853d8e41e82296d15f27750b2
@kassens
Copy link
Member

kassens commented Jun 13, 2019

This should be fixed in the next release thanks to #2764.

@kassens kassens closed this as completed Jun 13, 2019
@wyattanderson
Copy link
Author

All credit to @robrichard for tracking down the root cause!

@kassens any chance of a minor release in the near future? This is blocking us from upgrading past 1.7.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@kassens @wyattanderson @sibelius @robrichard