Warning is printed when optional modules are "missing"

[wavto]

System & Version

My MacBook Pro runs macOS Sierra 10.12.6, my college working on Ubuntu 17.10 has the same issue. It occurs (at least) with Meteor v1.5.2.1 v1.5.2.2 & v1.6.

Error Description

I added the npm package bootstrap-slider to our Meteor project, which has an optional dependency to jQuery. That means the package calls require("jquery") in an if statement, at some point.

Since we do not use jQuery in our project, I got the following warning every time the Meteor server restarts:

Unable to resolve some modules:

  "jquery" in /my/project/path/node_modules/bootstrap-slider/dist/bootstrap-slider.js (web.browser)

If you notice problems related to these missing modules, consider running:

  meteor npm install --save jquery

As @benjamn described here the Meteor ImportScanner statically searches for require(<string literal>) in the code base, without evaluating if that code is ever executed.

This issue affects all packages requiring optional dependencies. For example, that means all packages build on top of this jQuery plugin "boilerplate", like bootstrap-slider in my actual case.

Reproduction Recipe

# create a fresh meteor project
meteor create warn-issue
cd warn-issue

# install the bootstrap-slider npm package
meteor npm i -S bootstrap-slider

# import `bootstrap-slider` on the client's main.js
echo "import { Slider } from 'bootstrap-slider';" | cat - client/main.js | tee client/main.js

# run the app
meteor run

which produces this output:

[[[[[ /private/tmp/warn-issue ]]]]]

=> Started proxy.

Unable to resolve some modules:

  "jquery" in /private/tmp/warn-issue/node_modules/bootstrap-slider/dist/bootstrap-slider.js (web.browser)

If you notice problems related to these missing modules, consider running:

  meteor npm install --save jquery

=> Started MongoDB.
=> Started your app.

=> App running at: http://localhost:3000/

Solution Proposal

My expected behaviour is not to have this warning printed every time the Meteor server restarts.

I don't really understand what the purpose of this warning is – in case the "missing" module is really missing at execution time, the app will crash anyway and the developer will have to install the missing module to be able to run his code.

Anyway, if this warning is somehow important to keep, my proposition will be to add an option somewhere to be able to list "known missing modules". These modules should then be ignored by the ImportScanner or at least they should be considered before printing the warning.

What's you opinion?

[benjamn]

I don't really understand what the purpose of this warning is – in case the "missing" module is really missing at execution time, the app will crash anyway and the developer will have to install the missing module to be able to run his code.

Warning at build time (even if it's a false positive) seems better than waiting until runtime for the application to crash!

[wavto]

True, you're right!

What do you think about the proposal having the ImportScanner to somehow exclude "known missing modules"?

[hwillson]

@avanthay - we'd like to keep the build time warning for now (as per #9285 (comment)), but regarding:

my proposition will be to add an option somewhere to be able to list "known missing modules". These modules should then be ignored by the ImportScanner or at least they should be considered before printing the warning.

Would you mind opening a new feature request for this over in http://github.com/meteor/meteor-feature-requests/issues? Thanks!

[wavto]

Thank you for the reply, I agree on not entirely removing the warning, that would ends up in crashing apps.

I just opened this new feature request meteor/meteor-feature-requests#228 for this.

[jymbob]

Apologies for the 🧟 but as the feature requests repo is archived and this was never resolved, I'm proposing reopening.

Use case: We have an internal app framework with dozens of our own private components which are then imported when needed by one of our core apps. These are all stored in a componentMap.js which includes a lazy import for every one of the private components. This structure works well and keeps the individual core app bundle sizes down, however we get warnings in the terminal for every unresolved module at build time.

An "ignore list" for the ImportScanner would solve this quickly and easily.

[Grubba27]

Hey, @jymbob, this seems a fairly doable feature.
Would you like to take it?

cc: Any opinions, @denihs?

[jymbob]

@Grubba27 I'd be happy to take a look, but TypeScript and node aren't my usual set of tools.

From a quick scan of import-scanner.ts, it looks like we'd need to add an ignoreList to ImportScannerOptions and then check against it before returning this.onMissing - does that seem a good place to start?

[zodern]

Meteor's build system tries to be config free, so it would be nice if Meteor could detect these scenarios and not log the warning without configuration. In the past, I've only really seen this when require is inside of a try block, which findImportedModuleIdentifiers could detect.

I'm not sure I understand why this warning is shown with your use case. Do the files imported by componentMap.js exist? Where the components are used, are they imported from componentMap.js or imported directly?

[jymbob]

Simplified example follows. I'm very happy to hear alternative suggestions for how to handle this.

Framework modules:
ComponentHolder
Component1
Component2

componentMap.js (part of ComponentHolder module):

const Component1 = lazy(() => import('framework-component-1')
const Component2 = lazy(() => import('framework-component-2')

export const componentMap = {
  foo: {
    name: 'foo',
    component: Component1
  },
  bar: {
    name: 'bar',
    component: Component2
  }
}

All Core Apps use ComponentHolder to display components.

import { componentMap } from 'framework-component-holder/componentMap'
const ComponentHolder = ({componentName, ...props}) => {
  ...
  const Component = componentMap[componentName].component
  return (
    <Suspense fallback={<Loading/>}>
      <Component {...props} />
    </Suspense>
  )
}

Core App A uses Component1 and Component2 - its package.json has all three listed as dependencies
Core App B only uses Component1 - there is no mention of Component2 in its dependencies. It will never render Component2

Core App B prints a warning that Component2 is missing.

The thinking behind this structure is that it gives us quick and easy code reuse, and a single place to keep the mapping, while theoretically keeping the bundle size down by not including unwanted components.

[zodern]

Thanks @jymbob. Using dynamic imports for this makes sense. Two ideas of how we can improve this without adding configuration:

• Only warn once during the initial build when modules are missing, and wait until the list of missing modules changes before warning again, instead of doing it during every rebuild.
• Not warn when unable to find an npm package imported in a dynamic import. Without using require, this is the only other way of importing optional dependencies and this way that use case would be fully supported. Though this would hide some valid warnings.

[jymbob]

I'd vote for the former if that's an easy change. A reminder once that you've got something unusual going on is handy. A reminder every time starts to get ignored

[make-github-pseudonymous-again]

sharp is another example of a library with such issues, see lovell/sharp#4099. The bit of code that imports the "missing" dependency is indeed inside a try/catch.