Skip to content

Peer dependencies#4

Merged
javivelasco merged 2 commits intoreact-toolbox:masterfrom
nickpisacane:deps
Nov 7, 2015
Merged

Peer dependencies#4
javivelasco merged 2 commits intoreact-toolbox:masterfrom
nickpisacane:deps

Conversation

@nickpisacane
Copy link
Copy Markdown
Contributor

Awesome work on react-toolbox!
This example helped me get started quickly. I added node-sass, normalize.css, and react-addons-css-transition-group to devDependenices in package.json (webpack build was failing with out them), and README.md with some simple instructions. I figured for people using npm v3+ it would be useful to include the peerDependencies, as npm won't install them automatically.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your words! :)

Normalize should come as a dependency from Toolbox. The commons.scss includes normalize, it's failing the build without it for you? node-sass is ok there I think

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javivelasco yes, its failing without normalize.css, I've just tried to add only node-sass & react-addons-css-transition-group to devDependencies:

ERROR in ./app/style.scss
Module build failed: ModuleNotFoundError: Module not found: Error: Cannot resolve module '~normalize.css' in /Users/vyorkin/github/react-toolbox-example/app
    at Compilation.<anonymous> (/Users/vyorkin/github/react-toolbox-example/node_modules/webpack/lib/Compilation.js:228:38)

I'm on npm v3.3.6, it doesn't install peerDependencies.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the initial build fails without node-sass, and normalize.css for anyone using npm 3.0+, as peerDependencies will not be installed automatically. npm will warn the user of unmet peerDependencies, which I think is fine an npm installation. However, I think it would be useful for the example to include the peerDependencies as a devDependencies so people can simply clone, install, and run, regardless of which npm version is installed.

Here is the output after running npm i (npm v3.3.6)

npm WARN EPEERINVALID sass-loader@3.1.1 requires a peer of node-sass@^3.3.3 but none was installed.
npm WARN EPEERINVALID react-toolbox@0.11.3 requires a peer of react-addons-css-transition-group@^0.14.0 but none was installed.
npm WARN EPEERINVALID react-toolbox@0.11.3 requires a peer of normalize.css@^3.0.3 but none was installed.

On Ubuntu 15.04, the build failed without react-addons-css-transition-group, but did not on OSX. Currently, I'm trying to get to the bottom of that.

Thanks for looking into this!

@javivelasco
Copy link
Copy Markdown
Member

I see. I'm ok with that, merged!
Thanks!

javivelasco added a commit that referenced this pull request Nov 7, 2015
@javivelasco javivelasco merged commit 5d041f2 into react-toolbox:master Nov 7, 2015
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

Successfully merging this pull request may close these issues.

3 participants