Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Register Generators (as stubs) with resolved path #99

Merged
merged 6 commits into from
Jun 23, 2018

Conversation

MarcoScabbiolo
Copy link
Contributor

Added a resolved parameter to registerStub to allow the generator to be stored with a resolved path other than 'unknown'.

With this change it is possible to adapt yeoman-test/RunContext to run a Generator as a module together with the path associated to it. This is crucial for working with transpiled generators, as the code source is in a different path from the actually executed code path and there has to be a way to separate the generator itself from it's associated file system path to have a decent experience (eg: having coverage results from the source code, not the transpiled code). I'll make the PR to yemoan-test after this is merged.

@@ -244,9 +244,10 @@ class Environment extends EventEmitter {
*
* @param {Function} Generator - A Generator constructor or a simple function
* @param {String} namespace - Namespace under which register the generator
* @param {String} resolved - The file path to the generator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's mark this parameter as optional (let's apply everywhere)

* @param {String} [resolved] ...

lib/store.js Outdated
@@ -43,9 +44,9 @@ class Store {
});
}

_storeAsModule(namespace, Generator) {
_storeAsModule(namespace, Generator, resolved) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use a default argument here: resolved = 'unknown'

@SBoudrias
Copy link
Member

Looks good to me, just 2 small changes to make.

@SBoudrias
Copy link
Member

Thanks! :)

@SBoudrias SBoudrias merged commit bec6f47 into yeoman:master Jun 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants