agraboso / redux-api-middleware

Redux middleware for calling an API.
MIT License
1.49k stars 195 forks source link

isRSAA returning false - EntitySchema vs Schema? #1

Closed mohanzhang closed 9 years ago

mohanzhang commented 9 years ago

Thanks for this timely package - I finally got to the point a couple of days ago where I understood the "real world" redux example and wondered if anyone generalized the middleware - turns out you did!

I'm having a hard time getting a trivial action to be recognized as isRSAA. In the Chrome developer console, when I inspect the reason the function is returning false, it turns out that

> schema instanceof _normalizr.Schema
false

because schema in this context is actually a EntitySchema. I noticed that a new version of normalizr came out just a day ago, so I tried both version 1.0.0 and 0.1.3 but am having the same problem. Any ideas what I'm missing?

agraboso commented 9 years ago

I think the problem might be in your bundling. How are you doing it?

I just created a new branch called browsertest with a minimal test of this Schema vs. EntitySchema issue. Check out the branch, npm install, ./node_modules/.bin/webpack, go to the browsertest directory and open index.html on your browser. You should see a couple of trues in the console.

mohanzhang commented 9 years ago

Webpack is still black magic to me at this point, so sorry if this is a dumb question, but don't I need some config to run webpack? In fact, after the npm install, there wasn't even a webpack bin, so I did npm install --save-dev webpack which let me call /.node_modules/.bin/webpack, but I've only ever used it with a config file?

agraboso commented 9 years ago

The browsertest branch has webpack as a dev dependency, and a webpack.config.file in the root directory.

mohanzhang commented 9 years ago

Ugh, sorry, that's embarrassing. Late night brain fart. Yes, you're right. I verified that everything is working as you'd expect, so I think it's something that's part of my own local state.

I noticed that the normalizr I'm using is not the normalizr redux-api-middleware is using:

var _normalizr = __webpack_require__(328);

vs

var _normalizr = __webpack_require__(516);

So I just need to figure out how to get these two to be the same underlying module.

mohanzhang commented 9 years ago

Ok, the more I think about this, the more I'm convinced that this can't be a problem that is unique to my situation. Since redux-api-middleware has its own version of normalizr, when I require normalizr in my app, it will refer to a different instance of normalizr. The only way I was able to work around this was to change normalizr to a peerDependency in redux-api-middleware's package.json, like so:

  "peerDependencies": {
    "normalizr": "1.x"   // EDIT: changed this to 1.x to reflect the latest version of redux-api-middleware
  },
  "dependencies": {
    "isomorphic-fetch": "^2.1.1",
    "lodash.isplainobject": "^3.2.0"
  },

Sure enough, when I do this, the middleware correctly requires the same normalizr that I require from within my app, and then the schema instanceof Schema condition will return true, because it is the same underlying class definition from the same module (which was not the case before!).

Unfortunately, using peerDependencies results in these warnings

npm WARN peerDependencies The peer dependency normalizr included from redux-api-middleware will no
npm WARN peerDependencies longer be automatically installed to fulfill the peerDependency
npm WARN peerDependencies in npm 3+. Your application will need to depend on it explicitly.

So I'm not quite sure what to make of it.

agraboso commented 9 years ago

Try the peerdep branch. There I've declared normalizr as both a dependency and a peer dependency. That should solve the problem for now (at least until npm v3 comes out). Let me know if it does so I can merge it and publish to npm as 0.4.1. Make sure you're using normalizr >= 1.0.0 in your code.

agraboso commented 9 years ago

By the way, I think npm would still issue the warning, but don't worry about it as long as it works. I have to read up on this peer dependency hell...

mohanzhang commented 9 years ago

Well darn, I appear to have wasted your time Alberto :( ... But I have also wasted my own time, so we are still in the same boat! I kid, I kid... ;) But seriously, apologies. Here's the postmortem:

The good news is that the peerdep branch works. ... The bad news is that the 0.4.0 release works too.

At this point, I'm a bit fried from deleting things and cloning/installing starting from scratch, so I can't verify what exact action did it, but either upgrading npm (unlikely), redux-api-middleware now using normalizr 1.0 (possible), or doing something that caused webpack to bust its cache in a way that I don't understand (likely) got me out of the bad state that started this whole issue, and now I can't even reproduce the problem at all.

Maybe if someone else runs into this problem they will say something, but until then let's consider it solved.

agraboso commented 9 years ago

From your description, I'm pretty sure it is the fact that you are now using normalizr v1 instead of v0.1.3 in your code. It's a bit of a subtle point of semver: 1.0.0 doesn't satisfy ^0.1.3 — which is what you get in your package.json file by doing npm install --save-dev, so if you had ^0.1.3 in your code, webpack bundled two different versions of normalizr — v0.1.3 from you code, and 1.0.0 from redux-api-middleware.

From node-semver

Caret Ranges ^1.2.3 ^0.2.5 ^0.0.4

Allows changes that do not modify the left-most non-zero digit in the [major, minor, patch] tuple. In other words, this allows patch and minor updates for versions 1.0.0 and above, patch updates for versions 0.X >=0.1.0, and no updates for versions 0.0.X.

agraboso commented 9 years ago

Oh, and don't worry: you're not wasting my time. I'm not that experienced myself, and I enjoy learning about these little quirks.

mohanzhang commented 9 years ago

Yes, agreed. That's why I think it ultimately had something to do with busting webpack's cache (I am using webpack-dev-server). I had originally tried changing my app's normalizr dependency to 0.1.3 to match the middleware's dependency, but to my surprise, nothing had changed. It occurs to me now that one thing I did not do at the time was wipeout node_modules entirely like I have been doing to ensure a clean state where all the dependencies could resolve from scratch. In any case, I am glad it's over :)

agraboso commented 9 years ago

:+1:

Do keep me in the loop about how redux-api-middleware is working for you. If you publish your code, give me a holler: I'd love to see it.