Cannot resolve module react or react-dom webpack errors
mcMickJuice opened this issue · 30 comments
Following your examples and running npm start, I'm getting the errors below. Should react and react-dom be installed as peer dependencies instead of a dependency of enclave?
Error: Cannot resolve module 'react/lib/ReactMount' in M:\Dev\examples\enclave-demo\src
Error: Cannot resolve module 'react' in M:\Dev\examples\enclave-demo\src
Error: Cannot resolve module 'react-dom' in M:\Dev\examples\enclave-demo\src
enclave.js
exports.entry = "./src/Main.js"
exports.output = "dist"
exports.port = 8080
exports.index = "./src/index.html"
exports.live = false
Main.js
import React from 'react';
import { render } from 'react-dom';
class App extends React.Component {
render() {
return (
<div>
<h1>
Welcome to my app!
</h1>
</div>
);
}
}
render(<App />, document.getElementById('root'));
Hey @mcMickJuice are you on a Windows machine? I know sometimes node acts differently on windows, lemme this out on my windows box.
indeed i am on a windows machine. Sorry for not mentioning it. Adding react and react-dom as peer-dependency fixes this issue.
Ah you're right. I'd like enclave to be as PnP as possible, but having react as an enclave dep is a little bit weird.
Perhaps it might make sense to have enclave manually install react and react-dom as peer-deps to a users project.
I'm still ok with keeping babel and webpack as enclave dependencies since they're not really used outside of compiling.
I'll make an issue to add manually installing it as a peer-dep. Would you like to make a PR for it? The change would need to happen in the src/postinstall/index.js file. That's where all postinstall scripts get ran. I assume it would be similar to how we manually insert the npm start script into the package.json.
Let me know if you're not interested and I'll tackle it!
I just ran through the demo on Windows, worked great. #18 to fix Readme.md
Cool project!! I have been playing with React + TS, just using systemjs (no jspm). This is Awesomer.
@eanplatter I can make that change. Let me know when you create the issue and I'll get to it tonight or tomorrow
@eanplatter, why would you not just add React as a peer dependency to enclave itself? That way you could enforce that the user install React in their project root. Then we would just need to update the Readme to include react on its install line.
@AgentEpsilon currently enclave installs React as a dependency of itself, which works on OSX (and probably anything unix based) but not Windows.
I also don't like how when you get started with Enclave you have only enclave as a dependency, but you're using React. I'd like the user to have their package.json file to show their react and react-dom deps. Sometimes you want to do things like use an older version, so the user should be able to manipulate that.
But I do like the idea of Enclave doing that install for the user initially, just to limit the amount of steps needed to get started.
@mcMickJuice I will get an issue up with more details on how I think it could be done.
@mikekidder Thanks man! I am glad it worked for you, I always have a hard time testing on windows.
@eanplatter Ah, I see! So your idea would be to just run npm i -S react in the root directory with shelljs on postinstall then?
Edit: Or perhaps on _pre_install, so that NPM would recognize that react is already installed and not reinstall it for enclave?
@eanplatter why not just list these as peerDependencies of enclave and have npm do all the work instead of enclave modifying the project's package.json and/or running npm install commands? Is this future proofing against npm 3, which to the best of knowledge WILL NOT install peer dependencies?
Holy shit, I've never heard of the peerDependencies property. my bad: https://nodejs.org/en/blog/npm/peer-dependencies/
Thanks for putting up with my ignorance. Let's do that instead.
@eanplatter no worries! A concern is, though, that npm 3 doesn't automatically install peerDependencies. We could fix this by just making the install line in the readme install both enclave and react, i.e. npm i -S enclave react.
One thing is, I like the idea of having the dependency inside the user's package.json file, so that they can manage it themselves, do peer dependencies do that?
I guess that's why I am in favor of writing a post install script to do that manually, I'd like to keep the getting started as small as possible. I'm ok with doing "magic" if it reduces friction without increasing the user's ignorance. If that makes sense.
@eanplatter Yes, the great thing about peer dependencies is that they are less an instruction to npm to install the package, and more a requirement that the user installs the package manually. So it would be installed in the user's package.json.
Whats the consensus here then? Do we want to have react and react-dom as peer dependency and switch over when npm 3 is used more or do we have a post install script run that installs these deps after the user goes through the prompts?
Or, based on the breaking change that npm 3 introduces see here for details, do we place the onus on the user to install these packages, based on the warnings that they receive. (I don't support the third option at all, but had to present it)
You know, if there is a warning saying that peer deps are missing, that sounds reasonable.
Sometimes folks ignore peer dep warnings though, what do you think about adding a some kind of message to the end of the post install script to remind users that if they're using React they need to install the deps?
The thing is, peer dependencies are kinda targeted at modules with plugins - i.e. a webpack loader would have webpack as a peer dependency. Enclave, on the other hand, is not a react plugin - in fact, the module itself doesn't even require react anywhere. Which brings me to my actual point:
Enclave has no business depending on react at all.
Add it as a peer dependency if you like - but keep in mind that if the user decides, for some crazy reason, to not use react in their project, nothing will fail. The babel preset for react operates fine, even if no jsx is found. Therefore, I put forward that it should be entirely up to the user to install react - if they need it for their project, they can install it.
So I feel kind of torn on this.
On one hand, Enclave began as an easier way to make React apps, so in that sense it makes sense for it to sort of depend on React, even if it seems a little arbitrary.
On the other hand, simplicity brings stability. The more moving parts we have the more room for breaks we have.
I guess the real question is, could enclave grow to serve more than react apps? Angular2 maybe? Vanilla JS apps?
If it would be a valuable resource in other lights, I'm ok with making react less implicit.
If we want to keep it react-centric, I think it's inevitable that it depends on react.
Thoughts? I'm all for debate.
The goal of Enclave is to make compiling React apps easier. The main purpose of Enclave is to take away the unnecessary options and configuration of a react application.
If we try to overcomplicate things by adding too much features It'll defeat the purpose and just become another webpack. I loved using it because of its simplicity and out of the box goodness. Moving away from this would be doing everyone a disservice.
I agree with @eanplatter that Enclave should depend on react. React ecosystem plugins are another debate. For example including react-router in the box could be a good or bad idea. So I believe that this is the kind of discussion that we should be having.
Also trying to expand Enclave to other frontiers this early is a bad idea. We barely have a solid feature set for React itself so let's keep it zoned in for now.
I think when it comes to a dependency that is optional, we shouldn't install it.
For instance, react-router is not the only router out there, redux has a router, and you can do routing with just javascript, so adding react-router as a dep would be bloat.
So if React is the only thing enclave is for, then react is not an option, so it should be a dep.
If we decide React isn't the only way to use enclave, then I am also ok with adding a postinstall prompt asking if they're going to use this as a react application, and then going from there.
I am 100% sold on not having React as a dep of enclave itself because there have been times where I have needed to change my react version like when 0.14 came out and I still used 0.13 because I wasn't ready to update my router to 1.0. It was a mess. I would have hated to not be able to change my package version of React.
#46 for anyone who wants to grab it. Moving react and react-dom to peer deps
Enclave depends on react -- "A simpler way to compile React applications". I don't take it as a Universal Webpack builder tool... For that, go check out webpackbin
I agree with most of what you said @eanplatter
But often when one uses Enclave, it's on a new project. So one wouldn't have to worry about managing versions and integrating other versions of tools because it's a fresh app and therefore I believe using the newest react/react as a dep would be beneficial.
Oh yeah the dependencies work for now, but this ecosystem is moving at breakneck speed.
So say you make a new app with enclave now, and it all works.
But 3 months from now react comes out with v16 and it breaks your code. If it's going to take you a week to fix your code to work with v16 but you're on a tight deadline, using enclave all of the sudden painted you in a corner.
Whereas if you can manage the dependency of react yourself, you can hold of upgrading react until you've got the time to handle it.
Enclave should always try to keep it's deps up to date, which makes sense if it is the one consuming the deps, but if a user is consuming them they need control.
I see where you're coming from, but you can still use that enclave app even if a react came out, because it doesn't automatically update your app making it invalid. One would have to manually upgrade his/her app to do that. React shipping a new version won't break any apps. It's on the user if they upgrade enclave and concurrently update the deps. So in my view the user has control no matter what
Yeah, it's pretty common to blow your modules away and reinstall them, which would get newer versions.
Also, a lot of times you don't realize a new version is going to break your app until you've already upgraded. I've felt this pain too recently :D
Proposal
- Enclave no longer holds react / react-dom as dependencies.
- Users should be able to manage their dependencies themselves
- Enclave doesn't use react or jsx so they're technically not dependencies
- Enclave facilitates assisting the user in installing React if prompted.
- Enclave's purpose is to reduce friction in getting started with React, it is still React focused
- If a user is trying to set up an existing project, or a non react related project, it's not installed extra dependencies
If there's no serious issues with that we'll move forward in that direction. Thank you all for the valuable discussion.
Take a look at this repo.
It's sole purpose is to wrap superagent and promisify it. In the readme, it states explicitly that you need to have superagent as a dependency. I understand that's a contrived example but we have to assume the users of enclave know how to use npm (they installed enclave afterall).
I think no matter what is done, react and react-dom need to be explicit dependencies listed in the root package.json. The question is how we achieve that. I say we run npm install commands for the those two libraries and be done with it. That way we achieve the following:
- future proof against npm 3 and dropping of peerDeps
- automatically include react and react-dom in root package.json
- make it easier for the user
Fixed in #91