Repository: ProjectEvergreen/greenwood (js)

Issue:
node modules resolver should return a fallback path for package not located by `require.resolve`
## Type of Change
- [ ] New Feature Request
- [ ] Documentation / Website
- [x] Improvement / Suggestion
- [ ] Bug
- [ ] Other (please clarify below)

## Summary
As part of #917 , noticed that inadvertently when **puppeteer** was added to `dependencies`, all of a sudden certain packages would fail the [`getNodeModulesLocationForPackage` utility function look up](https://github.com/ProjectEvergreen/greenwood/blob/v0.24.2/packages/cli/src/lib/node-modules-utils.js#L6), and error out in the terminal.
```sh
{ locations: null }
Unable to look up punycode using NodeJS require.resolve.
Falling back to process.cwd()
=============

  TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
      at validateString (internal/validators.js:124:11)
      at Object.join (path.js:1110:7)
      at walkPackageJson (file:///Users/owenbuckley/Workspace/project-evergreen/repos/greenwood/packages/cli/src/lib/walker-package-ranger.js:202:44)
      at async walkPackageJson (file:///Users/owenbuckley/Workspace/project-evergreen/repos/greenwood/packages/cli/src/lib/walker-package-ranger.js:209:11)
      at async walkPackageJson (file:///Users/owenbuckley/Workspace/project-evergreen/repos/greenwood/packages/cli/src/lib/walker-package-ranger.js:209:11)
      at async walkPackageJson (file:///Users/owenbuckley/Workspace/project-evergreen/repos/greenwood/packages/cli/src/lib/walker-package-ranger.js:209:11)
      at async walkPackageJson (file:///Users/owenbuckley/Workspace/project-evergreen/repos/greenwood/packages/cli/src/lib/walker-package-ranger.js:209:11)
      at async file:///Users/owenbuckley/Workspace/project-evergreen/repos/greenwood/packages/cli/src/plugins/resource/plugin-node-modules.js:103:13
```

What's interesting is that there is no change in dependecies before / after moving **puppeteer** into `dependencies`?
```sh
# before
yarn why punycode
yarn why v1.22.5
[1/4] 🤔  Why do we have the module "punycode"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "punycode@2.1.1"
info Reasons this module exists
   - "_project_#whatwg-url#tr46" depends on it
   - Hoisted from "_project_#whatwg-url#tr46#punycode"
   - Hoisted from "_project_#ajv#uri-js#punycode"
   - Hoisted from "_project_#jsdom#tough-cookie#punycode"
info Disk size without dependencies: "48KB"
info Disk size with unique dependencies: "48KB"
info Disk size with transitive dependencies: "48KB"
info Number of shared dependencies: 0
✨  Done in 0.88s.

# after
yarn why punycode
yarn why v1.22.5
[1/4] 🤔  Why do we have the module "punycode"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "punycode@2.1.1"
info Reasons this module exists
   - "_project_#whatwg-url#tr46" depends on it
   - Hoisted from "_project_#whatwg-url#tr46#punycode"
   - Hoisted from "_project_#ajv#uri-js#punycode"
   - Hoisted from "_project_#jsdom#tough-cookie#punycode"
info Disk size without dependencies: "48KB"
info Disk size with unique dependencies: "48KB"
info Disk size with transitive dependencies: "48KB"
info Number of shared dependencies: 0
✨  Done in 0.88s.
```

## Details
Not sure why locations is null, but I suppose there's no reason we couldn't at least try and do something, like fallback to `process.cwd()` I suppose
```js
if (!nodeModulesUrl) {
  console.debug(`Unable to look up ${packageName} using NodeJS require.resolve.`);
  // we could / should at least try to provide a fallback here?
}
```

Maybe this issue is relevant? - https://github.com/ajv-validator/ajv/issues/1381

> Note: somewhat related, but I think the `import.meta.resolve` logic is actually broken... 🙈  😬 

Pull request:
Enhancement/issue 919 node modules location lookup fallback
<!--
## Submitting a Pull Request
We love contributions and appreciate any help you can offer!
-->

## Related Issue
resolves #919 

## Summary of Changes
1. Add a fallback location for `getNodeModulesLocationForPackage`
1. Update theme pack specs (_could_ be a breaking change?)

## TODO
1. [x] Fix Windows specs
1. [x] Maybe pre-test in local _node_modules_ first as part of next alpha?
1. [ ] Update docs?  (windows URLs <> file paths interop and awareness)

Patch:
diff --git a/packages/cli/src/lib/node-modules-utils.js b/packages/cli/src/lib/node-modules-utils.js
index 3113eed0..393d03c9 100644
--- a/packages/cli/src/lib/node-modules-utils.js
+++ b/packages/cli/src/lib/node-modules-utils.js
@@ -1,5 +1,6 @@
 import { createRequire } from 'module'; // https://stackoverflow.com/a/62499498/417806
 import fs from 'fs';
+import path from 'path';
 
 // defer to NodeJS to find where on disk a package is located using import.meta.resolve
 // and return the root absolute location
@@ -40,7 +41,8 @@ async function getNodeModulesLocationForPackage(packageName) {
     }
 
     if (!nodeModulesUrl) {
-      console.debug(`Unable to look up ${packageName} using NodeJS require.resolve.`);
+      console.debug(`Unable to look up ${packageName} using NodeJS require.resolve.  Falling back to process.cwd()`);
+      nodeModulesUrl = path.join(process.cwd(), 'node_modules', packageName); // force / for consistency and path matching);
     }
   }
 


Test patch:
diff --git a/packages/cli/test/cases/build.default.import-node-modules/build.default.import-node-modules.spec.js b/packages/cli/test/cases/build.default.import-node-modules/build.default.import-node-modules.spec.js
index 46bb82f3..5574e82d 100644
--- a/packages/cli/test/cases/build.default.import-node-modules/build.default.import-node-modules.spec.js
+++ b/packages/cli/test/cases/build.default.import-node-modules/build.default.import-node-modules.spec.js
@@ -3,7 +3,7 @@
  * Run Greenwood with and loading different references to node_module types to ensure proper support.
  * Sets prerender: true to validate the functionality.
  * 
- * Uaer Result
+ * User Result
  * Should generate a bare bones Greenwood build without erroring.
  *
  * User Command
diff --git a/packages/cli/test/cases/develop.plugins.context/greenwood.config.js b/packages/cli/test/cases/develop.plugins.context/greenwood.config.js
index b7934683..b3b0cec5 100644
--- a/packages/cli/test/cases/develop.plugins.context/greenwood.config.js
+++ b/packages/cli/test/cases/develop.plugins.context/greenwood.config.js
@@ -1,7 +1,7 @@
 // shared from another test
 import fs from 'fs';
-import { myThemePackPlugin } from '../build.plugins.context/theme-pack-context-plugin.js';
 import path from 'path';
+import { myThemePackPlugin } from '../build.plugins.context/theme-pack-context-plugin.js';
 import { ResourceInterface } from '@greenwood/cli/src/lib/resource-interface.js';
 import { URL } from 'url';
 
@@ -14,11 +14,11 @@ class MyThemePackDevelopmentResource extends ResourceInterface {
   }
 
   async shouldResolve(url) {
-    return Promise.resolve(url.indexOf(`/node_modules/${packageName}/`) >= 0);
+    return Promise.resolve(url.indexOf(`node_modules${path.sep}${packageName}`) >= 0);
   }
 
   async resolve(url) {
-    return Promise.resolve(url.replace(`/node_modules/${packageName}/dist/`, path.join(process.cwd(), '/fixtures/')));
+    return Promise.resolve(path.normalize(url).replace(`node_modules${path.sep}${packageName}${path.sep}dist`, 'fixtures'));
   }
 }
 
diff --git a/packages/cli/test/cases/theme-pack/greenwood.config.js b/packages/cli/test/cases/theme-pack/greenwood.config.js
index 9e0dd9ed..ae0d3417 100644
--- a/packages/cli/test/cases/theme-pack/greenwood.config.js
+++ b/packages/cli/test/cases/theme-pack/greenwood.config.js
@@ -1,6 +1,6 @@
 import fs from 'fs';
-import { myThemePack } from './my-theme-pack.js';
 import path from 'path';
+import { myThemePack } from './my-theme-pack.js';
 import { ResourceInterface } from '@greenwood/cli/src/lib/resource-interface.js';
 import { URL } from 'url';
 
@@ -14,11 +14,11 @@ class MyThemePackDevelopmentResource extends ResourceInterface {
 
   async shouldResolve(url) {
     // eslint-disable-next-line no-underscore-dangle
-    return Promise.resolve(process.env.__GWD_COMMAND__ === 'develop' && url.indexOf(`/node_modules/${packageName}/`) >= 0);
+    return Promise.resolve(process.env.__GWD_COMMAND__ === 'develop' && url.indexOf(`node_modules${path.sep}${packageName}/`) >= 0);
   }
 
   async resolve(url) {
-    return Promise.resolve(this.getBareUrlPath(url).replace(`/node_modules/${packageName}/dist/`, path.join(process.cwd(), '/src/')));
+    return Promise.resolve(path.normalize(this.getBareUrlPath(url)).replace(`node_modules${path.sep}${packageName}${path.sep}dist`, 'src'));
   }
 }
 
diff --git a/packages/cli/test/cases/theme-pack/theme-pack.develop.spec.js b/packages/cli/test/cases/theme-pack/theme-pack.develop.spec.js
index 010888e2..1667cd58 100644
--- a/packages/cli/test/cases/theme-pack/theme-pack.develop.spec.js
+++ b/packages/cli/test/cases/theme-pack/theme-pack.develop.spec.js
@@ -38,7 +38,7 @@ const expect = chai.expect;
 const packageJson = JSON.parse(await fs.promises.readFile(new URL('./package.json', import.meta.url), 'utf-8'));
 
 describe('Develop Greenwood With: ', function() {
-  const LABEL = 'Developement environment for a Theme Pack';
+  const LABEL = 'Development environment for a Theme Pack';
   const cliPath = path.join(process.cwd(), 'packages/cli/src/index.js');
   const outputPath = fileURLToPath(new URL('.', import.meta.url));
   const hostname = 'http://localhost';