material-ui-pickers: Using undocumented `externalDependencies` instead of `peerDependencies` potentially causing issues

Environment

Tech	Version
material-ui-pickers	1.0.0-rc.5
material-ui	1.0.0-beta.40
React	16.3.2
Browser	Chrome
Platform	Windows 10

Steps to reproduce

Use yarn workspaces and install the required peer dependency of date-fns@^2.0.0-alpha.7 inside my /workspace/vs-frontend package.
Also rely on other packages which require date-fns, but an earlier incompatible version (webpack-cli for example).
Because yarn uses hoisting, it is hoisting material-ui-pickers up to a root node_modules folder, and the other date-fns packages, but leaving our specific version of date-fns@^2.0.0-alpha.7 behind because there is no known link between material-ui-pickers and that specific package and version.

Expected behavior

Busy getting feedback about the issue specifically relating to material-ui-pickers here: https://github.com/yarnpkg/yarn/issues/5705

Basically the expected behaviour would be that these two modules remain aligned in the same node_modules level.

Actual behavior

Instead I now have this folder structure:

node_modules
  |- date-fns@1.29.0
  |- vs-worker
  |    +- node_modules
  |        +- date-fns@2.0.0-alpha.7
  |- vs-frontend
  |    +- node_modules
  |        +- date-fns@2.0.0-alpha.7
  |- concurrently
  |- material-ui-pickers
  +- listr-verbose-renderer

Which is problematic for material-ui-pickers resolving the correct date-fns version. And I get this error:

Error: Cannot find module 'date-fns/addDays'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:548:15)
    ...

I’m busy getting feedback, but it seems that perhaps setting this external dependency correctly as a peerDependency might fix it. externalDependencies is not documented anywhere in the NPM spec, so I’m unsure why it is being used here.

About this issue

Original URL
State: closed
Created 6 years ago
Comments: 15 (10 by maintainers)

Commits related to this issue

Merge pull request #525 from dmtrKovalenko/feature/optional-dependnencies [#369] Move to optionalDependencies instead of externalDependencies — committed to mui/material-ui-pickers by dmtrKovalenko 6 years ago

Most upvoted comments

@RedHatter thanks for sharing this info here, didn’t know that optionalDependencies is standardized field.

@dmtrKovalenko I think we should use that, instead of externalDependencies 😃

cherniavskii on Jul 16, 2018

@dmtrKovalenko that’s what I’ve done, I’ve officially included it in my package.json as a direct dependency. But because of the way yarn hoisting works in workspaces, and because I have other modules which have the date-fns@1.x library requirement (in their own package.json), it not seeing material-ui-pickers as requiring this specific version of date-fns@next, so it doesn’t “hoist” them together and keep them in the same /node_modules context.

You can see in great detail what I’m talking about, as we’ve been discussing this exact problem with the yarn team over here: https://github.com/yarnpkg/yarn/issues/5705#issuecomment-383388063

In a regular project it wouldn’t cause any issues, but in monorepos it is causing issues because of the way their algorithms work to preserve space. They would only ensure material-ui-pickers and date-fns@next will stay together if there is an officially defined link between them (peerDependencies).

So yea @cherniavskii , its not so much the issue of externalDependencies being there - but rather that peerDependencies is not being used instead.

lostpebble on Apr 23, 2018