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

Allow globs for manualChunks #2336

Closed
funkybob opened this issue Jul 18, 2018 · 12 comments · Fixed by #2831
Closed

Allow globs for manualChunks #2336

funkybob opened this issue Jul 18, 2018 · 12 comments · Fixed by #2831

Comments

@funkybob
Copy link

Currently manualChunks requires you to explicitly name each module you want in a chunk.

For my case, I would find it much more useful to be able to specify globs, instead, so I could, for instance, use:

    manuaChunks: {
        vendor: ['node_modules/**/*.js']
    }

to place all 3rd party packages in a vendor chunk.

@tivac
Copy link
Contributor

tivac commented Jul 18, 2018

A filtering function would be nice too, so any possible kind of filtering could happen.

manualChunks : {
    vendor : (id) => id.includes("node_modules/")
}

@guybedford
Copy link
Contributor

This is not possible as glob behaviours are not defined in the module resolver, therefore there is no way to make this a well-defined feature without it having edge cases causing bugs.

Also note that you are free to write code in Rollup configurations that can handle this.

@tivac
Copy link
Contributor

tivac commented Jul 19, 2018

Also note that you are free to write code in Rollup configurations that can handle this.

Would this be globbing the target directory within the rollup config, and then setting something like this:

manualChunks : {
    vendor : [ /* all files matched by glob */ ]
}

?

@guybedford
Copy link
Contributor

@tivac yes exactly.

So filtering is a nice concept, but currently manualChunks allow defining new entry points. That is, if I set manualChunks: ['./file-that-isnt-used-by-anything'] then that file is treated as included in the build (although treeshaking will still remove any code that doesn't have side effects).

We could consider reworking manualChunks to be simply a filtering. That is we run the build on the entry points, working out graph etc. Then we run through the modules included, and pull into a manual chunk anything from there. The difference though would be that defining manualChunks: ['react'] would be empty if nothing has import 'react', whereas currently we pull in react and then tree-shake it.

The difference between the two cases being, in the one we have the ids to run a filter on, and in the other, we have only the name before resolution, so globbing makes less sense in that id space.

@guybedford
Copy link
Contributor

Also it's quite nice to entirely use the load function as the file system abstraction for eg in-browser builds. Globbing requires a list function...

@tivac
Copy link
Contributor

tivac commented Jul 30, 2018

I think the fundamental issue for me was that I thought manualChunks was a filtering mechanism for files that were already going to be part of a bundle, not a way to create brand-new entry points containing files that may or may not overlap with the code dependency graph.

I'm sure there's a use-case for it working that way, but it's definitely not what I expected given any of the documented examples of manualChunks!

It also means that the glob suggestion up above would probably make for insanely slow builds, since rollup would have to tree-shake literally everything within the node_modules folder right?

@guybedford
Copy link
Contributor

@tivac yes exactly :)

I'd have thought manual chunks being defining of chunks rather than filtering was more common-sense.... but I'm totally open to changing this if you think it would be better as simply a filter. Then yes we can minimatch and glob away certainly. Perhaps it would just need a docs note.

(although the next thing is then it might be confusing why only manualChunks gets this support...!)

@tivac
Copy link
Contributor

tivac commented Jul 30, 2018

A docs note explaining what manualChunks is exactly is probably a good place to start, it seems like I might not be the only person with the faulty understanding of its true purpose.

@guybedford
Copy link
Contributor

@tivac not faulty - maybe a better viewpoint. I'm wondering if we should change manual chunks to work as a filter like you suggest. What do you think?

@tivac
Copy link
Contributor

tivac commented Jul 31, 2018

I think it'd be more intuitive and useful, but would be interested in hearing from folks who actually use it as it works today.

@funkybob
Copy link
Author

funkybob commented Aug 1, 2018

I've no objection to the existing function remaining as is [and perhaps the docs clarifying what its purpose is]... but I'd also like a way to control which bundle any package is output into. Perhaps I've just never got my head around how the existing machinery should be used for such purposes. I find too much of the docs seem to be written for people who already understand all the concepts and terms.

@lukastaegert
Copy link
Member

I implemented #2831 which will not allow you to use globs but allows you to define manualChunks as a function, giving you full control to tackle the use cases listed.

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

Successfully merging a pull request may close this issue.

4 participants