Environment.prototype.resolveModulePath() resolves only directories if no file extension specified. #33
Conversation
| @@ -400,7 +400,7 @@ Environment.prototype.resolveModulePath = function resolveModulePath(moduleId) { | |||
| moduleId = path.resolve(moduleId); | |||
| } | |||
|
|
|||
| return require.resolve(untildify(moduleId)); | |||
| return require.resolve(untildify(moduleId) + path.sep); | |||
There was a problem hiding this comment.
Should we make sure the file path we get here is not pointing to a file before adding a separator?
There was a problem hiding this comment.
Agreed, I was thinking about that as I was walking around today. require.resolve will only resolve to a .js file or a .json file? Also, aren't all generators meant to be scoped by directory?
There was a problem hiding this comment.
require.resolve will only resolve to a .js file or a .json file?
Yes
Also, aren't all generators meant to be scoped by directory?
Yes, but we can't assume this method is always receiving a folder. It'd make it pretty brittle
|
Please use a descriptive title for the PR and commit. |
Freyert
changed the title
Freyert
changed the title
|
Sorry for flipping it open and close so many times. Still trying to figure out a lot of GitHub stuff. I think the new revisions are good. Let me know if I the PR title or commit is too verbose. |
|
LGTM, thanks for taking care of this! |
|
One of your test is failing though, check Travis log for the details. |
1 similar comment
|
Yay I did it! Thanks for the patience. |
|
Awesome thanks! |
Environment.prototype.resolveModulePath() resolves only directories if no file extension specified.
This solves an issue where
package.jsonwould be resolved instead ofpackage/index.js. It forces all module resolutions to directories. Fix suggested by @SBoudrias.