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

Disable requiring sub-dependencies #111

Closed
klaasman opened this issue Apr 5, 2018 · 5 comments
Closed

Disable requiring sub-dependencies #111

klaasman opened this issue Apr 5, 2018 · 5 comments

Comments

@klaasman
Copy link
Contributor

klaasman commented Apr 5, 2018

It's possible to import every package inside the node_modules directory. This may lead to bugs when for example in the future a package removes a dependency which was accidentally imported by the app itself without explicitly installing it.

@EECOLOR
Copy link
Member

EECOLOR commented Apr 9, 2018

I like the idea, it should be pretty easy to imlement as a separate plugin, something like this:

  webCompiler.hooks.normalModuleFactory.tap(p, normalModuleFactory => {
    const importedPackages = { ...getFromPackage(), ...getFromBuildTool() }  
    normalModuleFactory.hooks.afterResolve.tap(p, data => {
      if (isLibraryRequest(data) && !importedPackages[data.rawRequest])
        throw new Error('...')

      return data
    })
  })

@EECOLOR
Copy link
Member

EECOLOR commented Apr 9, 2018

This is actually more tricky than it sounds for stuff where we import a specific file. It's for example ok to import @kaliber/build/lib/hot-module-replacement-client when we have @kaliber/build in our package.json.

https://github.com/webpack/enhanced-resolve/tree/v4.0.0 is a tricky library. We could do some manual string splitting, but there might be a chance that the library already provides some information.

@EECOLOR
Copy link
Member

EECOLOR commented Apr 10, 2018

It still allows us to import dependencies that are specifically for the server (like express).

@EECOLOR
Copy link
Member

EECOLOR commented Apr 10, 2018

Oh and this is blocked by #113

@EECOLOR
Copy link
Member

EECOLOR commented Feb 20, 2020

It would also cause problems with the @kaliber/build. We don't want people to add react as a dependency because @kaliber/build provides it. So in some case we do want to allow sub dependencies.

The same goes for the webpack-sources library, we never want another version than the one that comes with webpack.

The complexity that is introduced when adding these edge cases is not worth the benefit. On top of that, we don't have enough examples where this caused a problem.

@EECOLOR EECOLOR closed this as completed Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants