Refactor env.register logic (Fix #353)#354
Conversation
|
BTW, I sent the PR right now to follow up on the issue and some review.
edit, adding stuff to the todos:
|
test/env.js
Outdated
There was a problem hiding this comment.
I had to refactor this test to use regexp rather than plain text comparison as using Object.defineProperty in register does not order objects the same way in Node.js 0.8 and 0.10
(As a side effect, now this test can pass on Windows)
Relevant Travis.CI failed job: https://travis-ci.org/yeoman/generator/builds/11264918
|
Ok, I cleared the remaining points I had forgotten yesterday. I also added unit test for the Should be ready for revision 😋 |
lib/env.js
Outdated
There was a problem hiding this comment.
Is there perhaps some nicer way to expand this?
There was a problem hiding this comment.
nope, there's a feature request for it on node core issue tracker, but that's the best way right now.
There was a problem hiding this comment.
Though I would go with if (filepath[0] === '~') {
|
Any news on this? |
|
Coverage remained the same when pulling 390faf7dde6b522d9159d3b649ddfb46dc05e15d on SBoudrias:test-76 into 4b330bb on yeoman:master. |
|
Tests fail on node 0.8 |
|
Coverage remained the same when pulling dd8fa6406e856a604913cf3a828e5d29f838923f on SBoudrias:test-76 into 24d93b5 on yeoman:master. |
env.register now only accept the generator path as a String. This force the generator.resolved behavior to be consistent. At the same time, removed any check for `generator.raw`; this is an unexpected side effect and obfuscate the code for very little gain. Added env.registerStub method to allow adding stub generators as function to the environment. A check is made on the function passed to force stubbed generators to extend Base (this is done automatically). Some test mechanics needed to be fixed in order to pass. This shouldn't have changed the test logic.
|
@sindresorhus Yeah, I rebased yesterday, and I wasn't happy with how the failing test was resolved. I fixed that on the lastest. |
|
Reviewed. Thanks for rebasing, Simon! |
Refactor env.register logic (Fix #353)
|
Yay! Thanks! |
5 participants
env.registernow only accept the generator path as a String. This forcethe
generator.resolvedbehaviour to be consistent. At the same time,removed any check for
generator.raw; this is an unexpected side effectand obfuscate the code for very little gain.
Added
env.registerStubmethod to allow adding stub generators asfunction to the environment. A check is made on the function passed
to force stubbed generators to extend Base (this is done automatically).
Some test mechanics needed to be fixed in order to pass. This shouldn't
have changed the tests logic.
The motivation behind this change is explained in #353. Basically it's a follow up after the 1.0 release bug to update what felt to me like an error prone logic and to make sure
generator.resolvedvalue was consistent no matter what.