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

Webpack's require.context not being employed #35

Open
brianmhunt opened this issue Apr 5, 2017 · 3 comments
Open

Webpack's require.context not being employed #35

brianmhunt opened this issue Apr 5, 2017 · 3 comments

Comments

@brianmhunt
Copy link

Related/original report: pahen/madge#117

It appears Precinct does not work with webpack's dynamic loading with require.context (basically glob) so e.g.

var context = require.context(".", true, /\.js$/)
context.keys().forEach(context)

This ought to load all the modules with /.js$/ from the current directory (".") and subdirectories (that's what the true specifies).

It does not appear to do so, at least via Madge/node-dependency-tree, and I did not see any mention of it in the Precinct code.

Just a heads up.

🍻

@mrjoelkemp
Copy link
Collaborator

Thanks so much for reporting the issue @brianmhunt. We'd have to add a sniff for require.context in detective-cjs and then call out to either webpack's resolver or roll our own to evaluate require.context to yield all of the files to be required.

Any other ideas for making this work?

@brianmhunt
Copy link
Author

Thanks for the update @mrjoelkemp . I think that's pretty much it. I didn't see any exposed code in Webpack to allow use of require.context externally, and I did a quick search but didn't see any useful npm projects that might've already solved it.

The require.context prototype is:

require.context(directory, useSubdirectories = false, regExp = /^\.\//)

require.context ought to return

  1. a function that, when called, requires the given file (may be relative to the directory); and
  2. have a keys property that is a list of all the filenames that match.

The globbing ought to be pretty straightforward walk + grep.

The function could basically just be equivalent to a require call (possibly with "directory" prefixed to the argument).

Here's a quick mock-up of roughly what I think it'd need to do:

require.context = function(dir, includeSubdirectories, regex) {
    const contextualRequire = (file) => require(path.join(dir, file))
    contextualRequire.keys = someGlobFunction(dir, includeSubdirectories)
    if (regex) {
      contextualRequire.keys = contextualRequire.keys.filter((r) => regex.test(r))
    }
    return contextualRequire
} 

@mrjoelkemp
Copy link
Collaborator

Thanks for the thoughts! I wonder if the resolveToContext option is the path forward. I'm currently not using that when resolving files via webpack (there's a 3 argument form of the resolver where the first arg is the context).

If that resolver option were the key, this would get supported via a few changes:

  1. detective-cjs would need to support this special case https://github.com/dependents/node-detective-cjs/blob/4e31629f92bafa3df9be09f5ddb3bae1474156fa/index.js#L13 of require.context. It would then have to save the arguments via a non-string output format (or an encoded string if we want to avoid multiple types for elements of the array of dependencies that precinct returns).
  2. The (encoded) require.context arguments would need to get processed (turning on the resolveToContext flag) (likely within node-filing-cabinet) when we've detected that the coded data from step 1 above.

The painful part of pushing this through filing-cabinet is that the api of that module is meant to return a single string, not an array of dependencies that were generated from a single import (require.context call). As such, it might be worth putting this (either your solution or enhanced-resolve's) into detective-cjs proper.


I'm open to reviewing a pull request that gets the desired functionality in. My hesitation is that require.context and even dynamically requiring files (within a for loop) are not possible with ES6 imports – which we'll all eventually migrate to anyway. Not sure if it makes sense to put in the work for these dynamic requires, but I'm happy to review and merge if others feel strongly.

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