Let first getter win if one already exists. #106
Conversation
lib/entry.js
Outdated
| // Should this throw if hasOwn.call(this.getters, name)? | ||
| entry.getters[name] = getter; | ||
| if (hasOwn.call(this.getters, name)) { | ||
| throw new Error("A getter for " + name + " already exists"); |
There was a problem hiding this comment.
The message can be bikeshedded. I just put something in it.
There was a problem hiding this comment.
Any preference on the error message?
There was a problem hiding this comment.
Let's make it Identifier 'foo' exported more than once?
|
A follow up to this would be a unit test to ensure it throws. I'm not super up on tests here though so it may be easier for @benjamn to do the follow up test. |
lib/entry.js
Outdated
| entry.getters[name] = getter; | ||
| if (hasOwn.call(this.getters, name)) { | ||
| throw new SyntaxError("Identifier '" + name + "' exported more than once"); | ||
| } |
There was a problem hiding this comment.
If we could have this error thrown only during tests, and otherwise let the first getter win (silently), I think that would be better than always throwing, since this is a runtime exception enforcing a compile-time guarantee.
lib/entry.js
Outdated
| // Should this throw if hasOwn.call(this.getters, name)? | ||
| entry.getters[name] = getter; | ||
| this.getters[name] = getter; | ||
| } |
There was a problem hiding this comment.
punted on erroring for now, opting to at least let the first getter win.
jdalton
changed the title
This addresses a line item of #90.