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

importing npm sub-folders can confuse checkNpmVersions #29

Closed
brucejo75 opened this issue Sep 28, 2021 · 3 comments
Closed

importing npm sub-folders can confuse checkNpmVersions #29

brucejo75 opened this issue Sep 28, 2021 · 3 comments

Comments

@brucejo75
Copy link
Contributor

This had me confused for quite a while today...

Repro

I am using nanoid@2.0.1.

In my package I

import { checkNpmVersions } from 'meteor/tmeasday:check-npm-versions';

checkNpmVersions({'nanoid': '2.0.1'}, 'meris:base');

const nanoid = require('nanoid/generate');

Result

Error message output:

WARNING: npm peer requirements (for meris:base) not installed:
  - nanoid@2.0.1 not installed.

Read more about installing npm peer dependencies:
  http://guide.meteor.com/using-packages.html#peer-npm-dependencies

Discussion

I believe that modules-runtime scans all the code looking for all import / require statements. Then if fills out a file tree structure based on those scans.

If you only reference a subfolder in an import or require, e.g.

import 'packageName/subfolder`;

modules-runtime would be unaware of anything in node_modules/packageName it would only be aware of what is in node_modules/packageName/subfolder. So it cannot find node_modules/packageName/package.json.

Workaround

essentially you can whitelist package.json for checkNpmVersions just like dynamic import whitelisting:

if(false) {
  require(`packageName/package.json`)'
}

FIX?

Not sure how this could be fixed in this case? Maybe this should be documented?

@brucejo75 brucejo75 changed the title importing npm sub-folders can confuse check-npm-versions importing npm sub-folders can confuse checkNpmVersions Sep 28, 2021
@brucejo75
Copy link
Contributor Author

I think #30 would resolve this.

@brucejo75
Copy link
Contributor Author

@copleykj,
Thanks for the quick response!

@copleykj
Copy link
Member

Yeah, unfortunately there isn't much to do but document because of the way the build system works.

In all reality, this package was a bandaid and honestly this functionality deserves first class support within the build system and this package retired.

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

No branches or pull requests

2 participants