The generator does not use templates installed in node_modules #425

laat · 2020-10-08T20:04:56Z

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...

The text was updated successfully, but these errors were encountered:

derberg · 2020-10-09T07:36:33Z

@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 · 2020-10-09T07:57:13Z

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 · 2020-10-09T09:34:35Z

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 · 2020-10-14T07:33:28Z

@derberg I created a PR :)

derberg · 2020-10-14T08:03:03Z

@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 · 2020-10-27T15:16:50Z

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

laat · 2020-10-27T15:36:00Z

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

derberg · 2020-10-27T15:38:38Z

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

laat · 2020-10-27T15:57:07Z

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 · 2020-10-30T15:49:02Z

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

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)

> puppeteer@5.4.1 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 tsutils@3.17.1 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/html-template@0.15.3
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.

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)

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 tsutils@3.17.1 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/html-template@0.15.3
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.

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

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)

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 tsutils@3.17.1 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/html-template@0.15.3
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 · 2020-10-30T16:11:58Z

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

It still ignores the package-lock.json versions.

derberg · 2020-10-30T16:50:12Z

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 · 2020-12-30T00:24:04Z

This issue has been automatically marked as stale because it has not had recent activity 😴
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 ❤️

derberg · 2021-03-01T08:23:35Z

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

derberg · 2021-03-02T10:03:15Z

@laat hey, it took some time, but it is finally here #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 · 2021-03-03T08:40:25Z

🎉 This issue has been resolved in version 1.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

derberg · 2021-03-03T08:41:49Z

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!

laat added the bug Something isn't working label Oct 8, 2020

laat mentioned this issue Oct 12, 2020

Use require algorithm to resolve templates #426

Closed

derberg mentioned this issue Oct 30, 2020

The generate does not install template modules correctly if local #391

Closed

magicmatatjahu mentioned this issue Dec 29, 2020

Typescript support and in-memory generation #468

Open

github-actions bot added the stale label Dec 30, 2020

derberg mentioned this issue Jan 14, 2021

How to use pre-install template? #482

Closed

derberg mentioned this issue Feb 24, 2021

feat: enable npm 7 support and remove npmi #517

Merged

github-actions bot closed this as completed Mar 1, 2021

derberg removed bug Something isn't working stale labels Mar 1, 2021

derberg reopened this Mar 1, 2021

derberg closed this as completed in #517 Mar 3, 2021

asyncapi-bot added the released label Mar 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The generator does not use templates installed in node_modules #425

The generator does not use templates installed in node_modules #425

laat commented Oct 8, 2020 •

edited

Loading

derberg commented Oct 9, 2020

laat commented Oct 9, 2020

derberg commented Oct 9, 2020

laat commented Oct 14, 2020

derberg commented Oct 14, 2020

derberg commented Oct 27, 2020

laat commented Oct 27, 2020

derberg commented Oct 27, 2020

laat commented Oct 27, 2020

derberg commented Oct 30, 2020

laat commented Oct 30, 2020

derberg commented Oct 30, 2020

github-actions bot commented Dec 30, 2020

derberg commented Mar 1, 2021

derberg commented Mar 2, 2021

asyncapi-bot commented Mar 3, 2021

derberg commented Mar 3, 2021

The generator does not use templates installed in node_modules #425

The generator does not use templates installed in node_modules #425

Comments

laat commented Oct 8, 2020 • edited Loading

The bug

Expected behavior

The repro has a patch

derberg commented Oct 9, 2020

laat commented Oct 9, 2020

derberg commented Oct 9, 2020

laat commented Oct 14, 2020

derberg commented Oct 14, 2020

derberg commented Oct 27, 2020

laat commented Oct 27, 2020

derberg commented Oct 27, 2020

laat commented Oct 27, 2020

derberg commented Oct 30, 2020

laat commented Oct 30, 2020

derberg commented Oct 30, 2020

github-actions bot commented Dec 30, 2020

derberg commented Mar 1, 2021

derberg commented Mar 2, 2021

asyncapi-bot commented Mar 3, 2021

derberg commented Mar 3, 2021

laat commented Oct 8, 2020 •

edited

Loading