duojs / duo

A next-generation package manager for the front-end
3.42k stars 118 forks source link

Fixing empty entry source string #486

Closed dominicbarnes closed 9 years ago

dominicbarnes commented 9 years ago

I've made the code checking raw and src a little more specific and clear. I believe I introduced this bug when I used ESLint to remove == undefined from these spots.

Basically, I've made the code a bit longer, but it should be a lot easier to maintain. (this fixes #484 and adds a test so it won't regress again)

/cc @darsain

darsain commented 9 years ago

This will break when this.src or this.raw are null, or object from some reason. More robust would be flipping the equals and type checking a string, which doesn't leave any holes that might cause errors:

if (typeof this.raw !== 'string')

Or maybe they can be Buffer objects and you need just an undefined check? Dunno the internals, but if that's the case, rather do double equals null, which checks null and undefined (see double equality table). Dunno how eslit, but this is the only double equals usage where jshint doesn't complain, because it's that useful.

dominicbarnes commented 9 years ago

@darsain I've gone ahead and allowed == null by updating the ESLint config. This matches what we had before, which means it should still fix the underlying issue.

darsain commented 9 years ago

Any reason for not doing the most robust typeof this.raw !== 'string' type check?

dominicbarnes commented 9 years ago

@darsain Hmm, I'm open to that possibility. What I have here works, so I'm going to press on with merging it, we can change it later if it turns out to be a problem.

dominicbarnes commented 9 years ago

If you guys find a bug in this implementation, please submit a PR with a breaking test.