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

Add an option to create context from the webpack config, i.e. via a plugin #2783

Closed
mover96 opened this Issue Jul 19, 2016 · 15 comments

Comments

Projects
None yet
6 participants
@mover96

mover96 commented Jul 19, 2016

I'm submitting a feature request

Webpack version:
2.1.0@beta.15

Please tell us about your environment:
Windows 10

Current behavior:
When using System.import (I am using webpack 2), webpack uses its context system (https://webpack.github.io/docs/context.html) to determine the path. In my case it is impossible for me to determine the URL before runtime, as it is attached to a Window object by the server when it starts up. Before I switched to webpack, I was using SystemJs to import these files, and since every file was transpiled from typescript and was sitting on the disk, the system.import was fine because it would just navigate to that file on the disk. With webpack, however, it needs to know about these files before runtime so it knows what to bundle and how to set up the linking and whatever else it does.

The context system exist to somewhat circumnavigate this issue, but I find it very lacking. The way it is set up is it determines what that import statement could possibly be, and imports all of them preemptively. This was an issue at first since the import statement was System.import(path), so I switched that statement to provide more context for the context engine. However, I am trying to import 60-100 files this way (they are 5-10 line angular components), and the highest directory they all share is app/ itself. They are scattered throughout the entire app, as these are all companion components that provide documentation for all of the main components. This breaks down the context system, because aside from importing absolutely everything preemptively, it has no other options.

I restructured some of the app to collect these files in 4-5 folders throughout the app. Even with this, the only option webpack's context system gives is to have a System.import statement written with the appropriate context for each folder. For example:

System.import('path/to/folder/a/' + filename + '.ts').then();
System.import('totally/different/path/to/folder/b' + filename + '.ts').then();

This is a big issue because now what should be a single import statement is now 2 in the source, which would have to be wrapped in if/else statements and if anyone looked at this code would immediately think this code is extraneous and inefficient. And the other issue is I just had to change my source code to support a specific bundler which would have to be documented, and in 3 months when the next cool whatever.js comes out and we want to switch, we have code in our source that is specifically for webpack, which is not desired.

I understand this is a very complex issue, because I am asking webpack to bundle files that will not be determined until runtime, i.e. once webpack has finished bundling. However these System.import() are fully supported by the ES6 spec*, and so they will not be going away anytime soon, and especially in my codebase I expect to start seeing them more often with even more complicated/obscure paths.

*well maybe not fully I do not know the specifics of the spec (and don't exactly care to know) but they aren't going away.

Feature Request:
I would like to let webpack know about these files via a config, so that I do not have to alter my source code. If I could setup contexting via a plugin, so that webpack would know what to bundle, it could generate its map thing or whatever it does, and then associate that set of files with a specific System.import() (or maybe all of them with some option) then it could handle fairly complex pathing. Not only could you specify the exact file path, but you could also give the plugin the same context you give System.import() now and it would do what it already does and would grab all possible dependencies. Since I have these files scattered throughout my app in various folders, this kind of specificity is needed.

  • Browser: [all]
  • Language: [TypeScript]
@sokra

This comment has been minimized.

Member

sokra commented Jul 19, 2016

You are in luck, as there is already a plugin doing exactly this: The ContextReplacementPlugin.

It allows to configure a regexp like this:

new ContextReplacementPlugin(/selector/, "./folder", true, /filter/)

Or it allows to set the exact mapping (webpack 2):

new ContextReplacementPlugin(/selector/, "./folder", {
  "./request": "./request",
  "./other-request": "./new-request"
  /* runtime-request: compile-time request */
})
@mover96

This comment has been minimized.

mover96 commented Jul 19, 2016

I've read the docs on this plugin and I guess I am a little confused on how it actually works. What is the purpose of the "./folder" in your example? What if I have these resources in different folders? For example my understanding is I would set up a custom regex in my system import:

System.import(/group1/);

And then in the plugin, would it be setup something like this?

new ContextReplacementPlugin(/group1/, "./folder", {
   "/styleguide/moduleA" : "./actual/relative/path/to/moduleA",
   "/other/directory/moduleB" : "./actual/different/relative/path/to/moduleB"
})

Is this correct? And I still do not understand what the "./folder" is doing haha. Thanks for your help, glad to see that this is already in webpack!

@mover96

This comment has been minimized.

mover96 commented Jul 20, 2016

Ok after spending some time with it I understand how your first example is working, but how would I get it to work with something like this:

let path = '../../' + this.componentDetails.path + '/styleguide/' + sampleFileName;
System.import(path);

Where the path info is gotten from the window, so it has basically 0 context. How to I target just that system import (so that it isn't redirecting every resolve in my app)?

I figured out that using the first method, my plugin would look like this:

new webpack.ContextReplacementPlugin(/selector/, "./src/app/ui_elements/mixins/", true, /^\.\/.*.sample_.*\..*ts/)

And if I wanted multiple directories to resolve I would use multiple plugins with the same /selector/, correct? Since what I am importing is scatter throughout the app in like 10 different folder locations, with the only common parent being the app folder itself. So my remaining question is, how do I get the selector to work efficiently, since the path can resolve to many different folders?

Thank you for your help @sokra.

@TheLarkInn

This comment has been minimized.

Member

TheLarkInn commented Jul 20, 2016

Awesome. This is really relevant question for Angular2 users with the dynamic AppModule resolver. Will need to add this to documentation for sure.

Also @mover96, thank you for the thoughtful and detailed question. This really helps us solve/answer things. You deserve some 🎉 🎉 .

@mover96

This comment has been minimized.

mover96 commented Jul 20, 2016

I feel like what would solve my issue is the ability to use something like require.context() but with system import, so I could just use System.import.context().then() and apply my context on a per System.import basis. But then I would also need the ability to chain contexts together so I can specify multiple folders. So maybe the plugin is better, but I cannot for the life of me get the resourceRegExp to match. If anyone has a working example of this plugin I would love to see it.

@sokra

This comment has been minimized.

Member

sokra commented Jul 20, 2016

Here is an example: https://github.com/webpack/webpack/blob/master/test/configCases/context-replacement/System.import/webpack.config.js

It should match the absolute path of the directory. yes it's a bit weird. We better write a new plugin for context information with a better API.

@mover96

This comment has been minimized.

mover96 commented Jul 21, 2016

I'm going to summarize my findings in case anyone runs into this thread, and I will also explain why the current system, even with ContextReplacementPlugin is inadequate.

The Goal:

First let me better explain what exactly I am trying to accomplish. I have a 600 module core app, most of which are Angular 2 components. This app already takes around 90 seconds to build (even with the vendors cached with the dll plugin) so it is a medium sized app. The vast majority of components are in a folder of their own name, along with a styleguide folder. The styleguide folders contain 3 files, a sample component, and an html and scss file for the component.

On the server side (.NET Core) I search the entire file tree for these styleguide folders, and put all the sample component names and paths into one big object. There are about 150 of these components (these are not including in the original 600). That object gets attached to window and that is the end of the server side stuff. Before I incorporated webpack into the build, I used SystemJs to grab these component's paths off of the window and dynamically load them using Angular's DyanmicComponentLoader like so:

Snippet 1

let path = '/' + this.componentDetails.path + '/styleguide/' + sampleFileName;
System.import(path).then //omitted

The dynamic loading and displaying of these components is handled by an entirely separate Angular app. And since the files were on the disk and already transpiled in the same directory structure this worked perfectly.

How the ContextReplacementPlugin Works:

Now, here is how to use the ContextReplacementPlugin to accomplish the same goal with minimal source file changes (see the initial post as to why I do not want to change anything in my source, which still wasn't even accomplished with the plugin). Keep in mind that these folders are literally down just about every tree in the main app, and this is the only solution I discovered. My import statement now looks like the following:

Snippet 2

let path = this.componentDetails.path + '/styleguide/' + sampleFileName;
System.import('group1/' + path + '.ts').then //omitted

and my ContextReplacementPlugin is setup as follows:

Snippet 3

new webpack.ContextReplacementPlugin(/.*group1/, "../../", true, /^\.\/.*.sample_.*\..*ts/)

The ContextReplacementPlugin will override the contexting for anything that matches the regex of the first parameter, the resourceRegExp (see Note 1). This is useful as in some situations, for catching all instances of your app trying to import a specific dependency. I think this is actually the main purpose of this plugin (see Note 2). However, for my situation (and I am sure there are plenty of other situation where this is needed) I needed to be at the root of my app with the first section of the System.import parameter string (since that is the highest directory that all the sample files share). Here is the first major issue:

Issue 1:

The plugin only uses the first portion of the string in the System.import (i.e. before the first concatenation with a variable) to get the context.

So, if I leave the code how I had it in Snippet 1 (see Note 3), the app would try and get its context from the main app directory. Which isn't bad, until you add the plugin to the mix. Since the plugin matches off of the first string in the System.import statement, my selector MUST be /\.\.\/\.\.\// (or basically ../../ but with regex) which causes the ContextReplacementPlugin to recontext any import statement (see Note 4) that has ../../ in it, thus intercepting requests not from just this specific System.import, and causing a slew of missing modules.

So to circumvent this, I use the resourceRegExp as a unique identifier, so I am 100% certain that the plugin is only affecting this System.import. This group1/ "directory" doesn't even exist in my file structure, and that is fine because anything that gets context'd is changed to simply ./.

The next thing the plugin does is goes to where the second parameter, newContentResource points. This is relative to where the System.import statement is. This then goes through and, the if third parameter, newContentRecursive is set to true, recursively goes down the file structure looking for any files or folder that match the regex in the 4th parameter, newContextRegExp. Luckily for me the files I am looking for all start with sample_ so I am able to trace my entire file tree and registering all files with sample_ in their name with webpack. While this works, it significantly increased by build times (from 90 seconds to 450 seconds) and comes with a few issues itself.

The Remaining Issues:

Issue 2:

The path the gets registered with the plugin (i.e. whatever file path gets found with the newContentResource and newContextRegExp) must be the same exact string that your System.import ends up calling. So if the ContextReplacementPlugin finds ../../path/to/some/file/sample_01.ts, your System.import must call the exact same string since it is mapped to the module when it is bundled. This might not be a big deal if the other issues get taken care of, but this prevented me from using some pretty hacky alternatives to get this up and running.

Issue 3:

There is no way to "chain" multiple ContextReplacementPlugins together. This kind of goes back to the initial post, where my solution was to give enough context to the System.import to hit a certain directory, but I did not want to have to write 5 statements. It would be nice to be able to have the newContentResource be different with multiple instances of the plugin, so that I can help narrow down the search to just a few directories (and then I still don't have to add a file to a list everytime I add a new component). But if this was implemented then it would have issues with Issue 2 still since all the paths would have to start from the same place, which in my app is forced to be app/.

Issue 4:

This is the straw that broke the camel's back from my project. I have 2 of these System.import statements that are identical. But one is for opening the component in an isolated view. The point is by having the same statement, even with the same "unique identifier", webpack still had to search the entire app structure again looking for matches for the newContextRegExp. This had the same increase on my build time yet again, going from 90 seconds originally to around 800 seconds. Webpack can handle 600 dependencies at 90 seconds, but by loading 200~ with this discovery method causes the build time to skyrocket.

Summary:

I hope that this provides some more information on how to use the ContextReplacementPlugin to its fullest extent. I hope that this also shows some of the more fundamental issues with using the plugin in its current state to really be able to have webpack be compatible with these dynamic imports (which is a very difficult issue for a static bundler!). Here is a brief summary of the core functionality a revamped/new plugins should aim to provide:

  1. Target a specific System.import. Maybe even like require.context (see Note 5)
  2. Point the context engine to a new directory, and have it either discovery with regex or list the dependencies manually (practically already implemented)
  3. Being able to "chain" the plugin with instances of itself, so that you can lessen the depth of the search
  4. In order to use number 3, there needs to be a way to not need the exact string to be passed to the System.import that was registered with the plugin's map.
  5. If possible, make it so that if 2 System.imports are tagged with the same unique identifier (or however you target the statement, i.e. number 1 above), that webpack can just associate both statements with the same context instead of having to search the tree again.

I hope this was informative, please let me know if there are any mistakes or if any part is difficult to understand. I still have a hard time with some of the Javascript fundamentals, so it is possible that I misinterpreted the source. As you might be able to tell with this post I love documentation, so let me know if this would be beneficial to add elsewhere as well.

Notes:

  1. If you are having trouble with your paths matching this regex, try removing any slashes in the expression after the word you are matching for, i.e. I was having issues with /.*group1\// working even though it should match. Same with beginning and ending line queries, avoid using ^ and $.
  2. This is really useful if the webpack contexting system gets out of hand, see #87
  3. Webpack can't follow your context through variables (which is understandable) and needs to have relative pathing. So in order to get Snippet 1 to even activate the contexting system (and not throw the Critical dependencies warning) it would acutally be structered as follows:
    System.import('../../' + this.componentDetails.path + '/styleguide/' + sampleFileName).then (sorry I had to do this inline or MD restarts my numbering lol)
  4. I am not sure if the plugin actually intercepts all imports/requires, or only the ones that activate the contexting engine. Either way, I have another System.import that works fine with the default contexting engine and it was getting hijacked by the plugin and causing errors.
  5. This might seem stupid to someone who knows what they are doing, but I had the great idea of using require.context to set up contexting my way, and then passing the path to the req object it returns. This worked great, but then I cannot use the System.import which I want to use for the chunking and dynamic loading. So I though trying something like this would work: System.import(req(path)). It does not for obvious reasons, since you cannot import an already imported module.
@TheLarkInn

This comment has been minimized.

Member

TheLarkInn commented Jul 21, 2016

First and foremost. I cannot thank you enough for this write-up and the time you took to explain your scenario. Not only is this an invaluable learning source for other developers, but contains some very awesome suggestions and enhancements that we can consider for webpack and ContextReplacementPlugin.

Also we'd like to use this writeup for our documentation ( maybe some tweaks to cater to multiple user bases)?

If you ever need help navigating the webpack source and you are wanting to contribute please let me know and I will be happy to guide you. 👏👏👏👏👏👏👏👏👏👏👏👏👏👏

@mover96

This comment has been minimized.

mover96 commented Jul 21, 2016

I am glad that it is useful! Feel free to use the write up and make whatever tweaks necessary. I was onboarded to a new company 3 weeks ago, and my first task was to get their preexisiting Angular 2 app working with webpack. I've spent almost 80 hours with it now (I am somewhat new to Angular as well, which explains some of the extraneous time) but webpack is a beast to get up and running. But, every single issue I have been able to figure out up until this one since there was almost no information that I could find about it. But I have come to learn how helpful posts like these can be haha!

That being said, I do feel like I have a pretty good grasp on webpack now, especially coming from an Angular 2 + some non-node backend view (which is a touch more rare than the React/node crew). And since I've had to explain a lot of my finding to my supervisor I seem to know where the common disconnects are for people. So long story short I would love to contribute to some of the documentation, because even though I've dumped hours into figuring out webpack, the end results are amazing. Our page reload speed went from 20+ seconds to 2, and the HMR is a godsend haha. So if you point me in the right direction I'll see what I can do!

@TheLarkInn

This comment has been minimized.

Member

TheLarkInn commented Jul 21, 2016

I'll ping you in the very near future. Thank you so so so so so so so so so so so so much.

@TheLarkInn TheLarkInn self-assigned this Jul 27, 2016

@datapimp

This comment has been minimized.

datapimp commented Aug 2, 2016

Thanks @mover96 for writing this up. I think the resolver / context system is by far one of the most interesting features of Webpack and I have also been struggling with the exact same things you described.

The solution I have wandered towards involved developing my own registry of file paths, defining regex style routes, using path-to-regexp to turn each of these paths into objects that contained metadata and other details which could be inferred by their file name / folder name and by parsing the file / analyzing the AST

e.g.

category('Components', {
  routes: [
    'components/:name.:extension',
    'components/:name'
  ],
  supportingFiles: {
    test: /\.spec\.js$/,
    example: /\.example\.md/
  }
})

This allowed me to build a database of tons of React components each which have accompanying files such as stylesheets, tests, documentation, examples. So I could do something like:

const layout = skypager.components.find({
  type: 'layout',
  name: 'multi-column'
})

assert( layout.dependencies.react, 'Layout components should import react')
assert( layout.hasDocBlock, 'Layout components should have ESDoc Headers')

or query all components by certain patterns to find out information about them

const connectedComponents = skypager.components.filter(
  component => component.dependsOn('react-redux')
)

connectedComponents.map(component => component.propTypes)

Using these types of queries, my desired goal was to be able to use webpack to be able to bundle them up for certain purposes -- whether it was to use it in production or to generate a kitchen sink gallery type page, or to find all of the existing components which lacked documentation or tests and auto-generate these files with useful content

The constraints you describe above have made my attempts less than stellar.

I ended up using Webpack's resolver plugins to take a require or import statement, used the request data Webpack provides, and used that to infer the intended module / file that the user was requesting and handled the resolution on my own using the metadata I gathered. I then passed webpack the absolute path and whatever loader information was required.

I still have a lingering suspicion that my solution could be much much cleaner by using a context replacement plugin but after reading your write up I am not sure. In either case I would love to take another crack at it and use more of webpack's internals to do it rather than rely on my own hacky and slow version.

Regardless, @TheLarkInn I would love to tag along if you ever do a walk through of Webpack's code base because I feel after this battle I could clean up a little and jump right into contributing.

I'd basically trade doing a bunch of chores on Webpack for some enlightenment.

@mover96 I'd love to see your project as well because it sounds very interesting

BTW:

An interesting project to check out is the https://github.com/yuanyan/haste-resolver-webpack-plugin

This provides an experience similar to react-native using facebooks internal @providesModule system which works by parsing the docblock . This allows you to move a file anywhere you like and still be able to require it without having to update all of the dependencies which use relative requires, which I gather is similar to what you are after.

@TheLarkInn

This comment has been minimized.

Member

TheLarkInn commented Aug 11, 2016

@datapimp I just saw this!!! Awesome. Would you also consider writing documentation for this as well? And yes, any time anywhere you let me know when you'd like to walk the webpack code. (But it may cost you some documentation 😏)

@datapimp

This comment has been minimized.

datapimp commented Aug 11, 2016

@TheLarkInn ha man i was feeling salty for a minute!

@TheLarkInn

This comment has been minimized.

Member

TheLarkInn commented Aug 11, 2016

Sorry lots of things thrown my direction these days and no good way to see the notifications. But yes just let me know. Context and the parser are two very high priority learning topics that if documented could really be useful to the community. So the more people I can try to teach the better.

@pksjce

This comment has been minimized.

Contributor

pksjce commented Sep 7, 2016

This issue was moved to webpack/webpack.js.org#140

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment