Generator: The generator does not use templates installed in node_modules

[laat]

The bug

This installs @asyncapi/html-template twice.

npm i -D @asyncapi/generator  @asyncapi/html-template
./node_modules/.bin/ag src/simple.yaml @asyncapi/html-template --force-write 

repro: https://github.com/laat/repro-asyncapi-generator

Expected behavior

It should reuse installed templates. This lets me pin templates at a specific version

The repro has a patch

This solves the issue for me. Essentially require.resolve does the heavy lifting and should support the intended uses of the replaced code.

diff --git a/node_modules/@asyncapi/generator/lib/generator.js b/node_modules/@asyncapi/generator/lib/generator.js
index 841af20..dc62139 100644
--- a/node_modules/@asyncapi/generator/lib/generator.js
+++ b/node_modules/@asyncapi/generator/lib/generator.js
@@ -320,31 +320,12 @@ class Generator {
     return new Promise(async (resolve, reject) => {
       if (!force) {
         try {
-          let installedPkg;
-
-          if (isFileSystemPath(this.templateName)) {
-            const pkg = require(path.resolve(this.templateName, PACKAGE_JSON_FILENAME));
-            installedPkg = require(path.resolve(DEFAULT_TEMPLATES_DIR, pkg.name, PACKAGE_JSON_FILENAME));
-          } else { // Template is not a filesystem path...
-            const templatePath = path.resolve(DEFAULT_TEMPLATES_DIR, this.templateName);
-            if (await isLocalTemplate(templatePath)) {
-              // This "if" is covering the following workflow:
-              // ag asyncapi.yaml ../html-template
-              // The previous command installs a template called @asyncapi/html-template
-              // And now we run the command again but with the resolved name:
-              // ag asyncapi.yaml @asyncapi/html-template
-              // The template name doesn't look like a file system path but we find
-              // that the package is already installed and it's a symbolic link.
-              const { resolvedLink } = await getLocalTemplateDetails(templatePath);
-              log.debug(`This template has already been installed and it's pointing to your filesystem at ${resolvedLink}.`);
-            }
-            installedPkg = require(path.resolve(templatePath, PACKAGE_JSON_FILENAME));
-          }
-
+          const pkgJsonPath = require.resolve(path.join(this.templateName, PACKAGE_JSON_FILENAME))
+          const installedPkg = require(pkgJsonPath)
           return resolve({
             name: installedPkg.name,
             version: installedPkg.version,
-            path: path.resolve(DEFAULT_TEMPLATES_DIR, installedPkg.name),
+            path: path.dirname(pkgJsonPath),
           });
         } catch (e) {
           // We did our best. Proceed with installation...

[derberg]

@laat Hey, I need more details to fully understand the issue.

• What is the use case for using generator like this npm i -D @asyncapi/generator @asyncapi/html-template ./node_modules/.bin/ag src/simple.yaml @asyncapi/html-template --force-write
• This lets me pin templates at a specific version what do you mean, now you can use a specific version of the template too, it is described here https://github.com/asyncapi/generator#generator-version-vs-template-version
• the change that you suggested, in what scenarios did you test it? did you also test scenario where you do not install a remote template but use it from local sources? it is a common scenario used during development

[laat]

At the moment, when installing @asyncapi/html-template puppeteer is installed which is about 130 mb. With my workflow it gets installed twice.

I want reproducible builds, and to do that I need all dependencies declared in pacakge-lock.json. By specifying a version on the commandline the build can fail the next day, without me changing anything, because a transitive dependency broke.

The code change I've suggested should work with npm link, because it uses the nodejs require mechanism which supports it. When reading the code I replaced, it looks like a partial implementation of the require algorithm. I have not tested the symlink case.

[derberg]

I want reproducible builds, and to do that I need all dependencies declared in pacakge-lock.json

ok, that makes sense

would you mind opening a PR, I'd love to help with testing different scenarios. I would work on your PR for a couple of days to make sure all works well

[laat]

@derberg I created a PR :)

[derberg]

@laat yeap, noticed. Will write down all possible test scenarios that need to be checked and starting from that point we will see how to approach testing. I don't want to throw responsibility for writing missing tests on you, but if you want to work on them, I will not stop you 😄

[derberg]

@laat wouldn't it be better for you to just install with PUPPETEER_SKIP_CHROMIUM_DOWNLOAD="true" and avoid duplicated chromium installation?

[laat]

It still does not solve the reproducible builds issue. Because package-lock.json is ignored.

[derberg]

@laat are you 100% sure it is ignored? the installation logs show 200 or rather different code (302 I think)?

[laat]

Yes.

First setting up the repository https://github.com/laat/repro-asyncapi-generator

export PUPPETEER_SKIP_CHROMIUM_DOWNLOAD="true"
npm ci
./node_modules/.bin/ag src/simple.yaml @asyncapi/html-template --force-write

Then see that it should install buffer 5.6.0 exactly:

Observe that it actually installs buffer 5.6.1:

[derberg]

you know what, I did some tests locally and yeah, I decided to first write down supported scenarios:

1. local html-template without node_modules and local generator without html-template in node_modules

   ./cli.js test/docs/dummy.yml ../html-template --force-write -o output
   
   npm http fetch GET 304 https://registry.npmjs.org/puppeteer 3836ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/rimraf 5198ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/@asyncapi%2fgenerator-filters 6777ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/lodash 103ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/openapi-sampler 170ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/markdown-it 174ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/argparse 100ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/mdurl 138ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/entities 150ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/linkify-it 279ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/uc.micro 389ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/sprintf-js 97ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/json-pointer 149ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/foreach 148ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/extract-zip 127ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/devtools-protocol 133ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/https-proxy-agent 179ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/debug 182ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/node-fetch 181ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/pkg-dir 236ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/tar-fs 240ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/ws 113ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/progress 360ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/proxy-from-env 453ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/unbzip2-stream 452ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/ms 103ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/yauzl 125ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/get-stream 134ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/@types%2fyauzl 142ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/@types%2fnode 102ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/pump 92ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/end-of-stream 85ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/once 234ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/wrappy 148ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/buffer-crc32 98ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/fd-slicer 132ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/pend 133ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/agent-base 196ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/find-up 117ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/locate-path 93ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/path-exists 97ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/p-locate 99ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/p-limit 130ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/p-try 143ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/glob 78ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/inflight 117ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/minimatch 136ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/path-is-absolute 144ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/inherits 162ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/fs.realpath 164ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/brace-expansion 102ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/balanced-match 107ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/concat-map 135ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/mkdirp-classic 110ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/chownr 128ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/tar-stream 128ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/bl 133ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/fs-constants 151ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/readable-stream 173ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/buffer 148ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/base64-js 86ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/ieee754 93ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/util-deprecate 110ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/string_decoder 149ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/safe-buffer 160ms (from cache)
   npm http fetch GET 304 https://registry.npmjs.org/through 74ms (from cache)
   
   > [email protected] install /Users/wookiee/sources/html-template/node_modules/puppeteer
   > node install.js
   
   **INFO** Skipping browser download. "PUPPETEER_SKIP_CHROMIUM_DOWNLOAD" environment variable was found.
   npm WARN [email protected] requires a peer of typescript@>=2.8.0 || >= 3.2.0-dev || >= 3.3.0-dev || >= 3.4.0-dev || >= 3.5.0-dev || >= 3.6.0-dev || >= 3.6.0-beta || >= 3.7.0-dev || >= 3.7.0-beta but none is installed. You must install peer dependencies yourself.
   
   + @asyncapi/[email protected]
   added 67 packages from 88 contributors in 16.511s
   
   65 packages are looking for funding
     run `npm fund` for details
   
   Done! ✨
   Check out your shiny new generated files at /Users/wookiee/sources/generator/output.
   

2. local html-template without node_modules and local generator with html-template in node_modules

   ./cli.js test/docs/dummy.yml ../html-template --force-write -o output
   
   Something went wrong:
   Error: Cannot find module 'rimraf'
   Require stack:
   - /Users/wookiee/sources/html-template/hooks/01_removeNotRelevantParts.js
   - /Users/wookiee/sources/generator/lib/hooksRegistry.js
   - /Users/wookiee/sources/generator/lib/generator.js
   - /Users/wookiee/sources/generator/cli.js
       at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
       at Function.Module._load (internal/modules/cjs/loader.js:725:27)
       at Module.require (internal/modules/cjs/loader.js:952:19)
       at require (internal/modules/cjs/helpers.js:88:18)
       at Object.<anonymous> (/Users/wookiee/sources/html-template/hooks/01_removeNotRelevantParts.js:3:16)
       at Module._compile (internal/modules/cjs/loader.js:1063:30)
       at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
       at Module.load (internal/modules/cjs/loader.js:928:32)
       at Function.Module._load (internal/modules/cjs/loader.js:769:14)
       at Module.require (internal/modules/cjs/loader.js:952:19)
   

3. local html-template with node_modules and local generator without html-template in node_modules

   ./cli.js test/docs/dummy.yml ../html-template --force-write -o output
   
   npm WARN [email protected] requires a peer of typescript@>=2.8.0 || >= 3.2.0-dev || >= 3.3.0-dev || >= 3.4.0-dev || >= 3.5.0-dev || >= 3.6.0-dev || >= 3.6.0-beta || >= 3.7.0-dev || >= 3.7.0-beta but none is installed. You must install peer dependencies yourself.
   
   + @asyncapi/[email protected]
   added 1 package from 1 contributor in 5.747s
   
   85 packages are looking for funding
     run `npm fund` for details
   
   
   
   Done! ✨
   Check out your shiny new generated files at /Users/wookiee/sources/generator/output.
   

4. local html-template without node_modules and local generator without html-template in node_modules

./cli.js test/docs/dummy.yml ../html-template --force-write -o output -i

result like step 1

1. local html-template without node_modules and local generator with html-template in node_modules

./cli.js test/docs/dummy.yml ../html-template --force-write -o output -i

/Users/wookiee/sources/generator/lib/utils.js:38
  const [nameWithVersion, pkgPath] = result[result.length-1];
                                                   ^

TypeError: Cannot read property 'length' of undefined
    at utils.beautifyNpmiResult (/Users/wookiee/sources/generator/lib/utils.js:38:52)
    at /Users/wookiee/sources/generator/lib/generator.js:373:17
    at /Users/wookiee/sources/generator/node_modules/npmi/npmi.js:164:37
    at /Users/wookiee/.nvm/versions/node/v14.15.0/lib/node_modules/npm/node_modules/graceful-fs/graceful-fs.js:123:16
    at FSReqCallback.readFileAfterClose [as oncomplete] (internal/fs/read_file_context.js:63:3)

1. local html-template with node_modules and local generator without html-template in node_modules

./cli.js test/docs/dummy.yml ../html-template --force-write -o output -i

npm WARN [email protected] requires a peer of typescript@>=2.8.0 || >= 3.2.0-dev || >= 3.3.0-dev || >= 3.4.0-dev || >= 3.5.0-dev || >= 3.6.0-dev || >= 3.6.0-beta || >= 3.7.0-dev || >= 3.7.0-beta but none is installed. You must install peer dependencies yourself.

+ @asyncapi/[email protected]
added 1 package from 1 contributor in 5.558s

85 packages are looking for funding
  run `npm fund` for details

Done! ✨
Check out your shiny new generated files at /Users/wookiee/sources/generator/output.

The question would be what is wrong with html-template symlink in generator in node_modules that makes scenario 2 not work the same as scenario 1. And how come can this require https://github.com/asyncapi/generator/blob/master/lib/generator.js#L329 trigger scenario 1 to work.

now some other question, did you try just this:

npm i -D @asyncapi/generator  @asyncapi/html-template
./node_modules/.bin/ag src/simple.yaml node_modules/@asyncapi/html-template --force-write 

[laat]

Cool, it's good to have the use cases documented. 👍

It still ignores the package-lock.json versions.

[derberg]

Ok, right, I did npm cache clean --force.

I created a project where I installed latest generator and old version of html-template without pdf generation support:

  "dependencies": {
    "@asyncapi/generator": "^1.0.0-rc.13",
    "@asyncapi/html-template": "^0.13.0"
  }
}

I run ./node_modules/@asyncapi/generator/cli.js http://bit.ly/asyncapi ./node_modules/@asyncapi/html-template --force-write -o output -p pdf=true

and I got error but also the template was downloaded fresh, with lots of 200s

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity :sleeping:
It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions :heart:

[derberg]

come one bot, can't you see I'm working on it, doh!

[derberg]

@laat hey, it took some time, but it is finally here https://github.com/asyncapi/generator/pull/517
if you want and have time, please have a look at this PR

your use case will be fixed + I've added the integration tests that we were always missing

[asyncapi-bot]

:tada: This issue has been resolved in version 1.2.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[derberg]

Release 1.2 enables support for latest node and npm and fixes all known installation issues. In case you see some other issues, please create a new issue. Please use --debug flag when you experience issues with the generator and in the issues mention the troubleshooting logs. Thanks!