systemjs / builder

SystemJS build tool
MIT License
465 stars 122 forks source link

Handling imported directories during bundling #501

Closed klausbayrhammer closed 8 years ago

klausbayrhammer commented 8 years ago

Hi,

We are using a system-js plugin to resolve the URL of an import, which basically works like this:

import urlToImageFolder from '../images!url' // returns e.g. http://localhost:8080/jspm_packages/registry/myPkg@1.0.0/images

This works totally fine as long as the app isn't bundled. When I bundle the app I tried to remove the imports which are using the url-resolver using the bundling arithmetics, which looks like this:

app.js - [registry:myPkg*/images!url]

Unfortunately this didn't work and the registry:myPkg@1.0.0/images!url was added to the bundle config.

Interesting thing is that the following config works and the dependency isn't added to the bundle config:

app.js - [registry:myPkg@1.0.0/images!url]

Same goes for importing (existing) files. If I change the import to import urlToSpecificImage from '../images/play.svg!url' and change the bundling expression to app.js - [registry:myPkg*/images/*.svg!url] everything works fine.

For the sake of completeness the (pretty simple) url-plugin looks like this:

const fetch = () => '';
const instantiate = load => load.address;
const bundle = () => '';

export {
    fetch,
    instantiate,
    bundle
};

Is there a way how I can avoid that imported directories are bundled? Probably by changing the plugin's bundle function? Because I don't think changing the nodir: true in the arithmetic.js is a solution - because you would have to exclude them in positive arithmentics and include them in negative arithmetics or something like that

guybedford commented 8 years ago

Can you clarify why you think nodir: true is not the solution here? It sounds like that is exactly the problem at least?

klausbayrhammer commented 8 years ago

Maybe you're right - if you specify your globs correctly it shouldn't be a problem at all. I was just thinking of cases where you specify something like app/**/* and all the directories would be added to the bundle config. But if it's as easy as changing the nodir option I'm totally satisfied :)

guybedford commented 8 years ago

Right so the intention is to exclude directories themselves from the glob results, but we still want globbing to glob directory variations like glob('dir*/*.js')? to include the directory variations. I would have though the normal glob even with nodir: true would have expanded these already? Is that definitely not the case?

klausbayrhammer commented 8 years ago

Given the following directory structure

app
|- x
| |- x.js
|- y

The following globs produce following results: glob('a*/**/*', {nodir:false}) -> [ 'app/x', 'app/x/x.js', 'app/y' ] glob('a*/**/*', {nodir:true}) -> [ 'app/x/x.js']

I'm pretty sure that the second option is way better for most people and that this should stay the default. Anyway it would may be an option to add a parameter to the bundling command whether the globs should expand to directories as well? That way you have an elegant way of excluding/including directories in you bundle config, but then you have to make sure that your globs are correct.

FYI - Our solution for now is to expand the globs ourselves and use the expanded globs without wildcards for constructing our bundles

guybedford commented 8 years ago

I've added a fix in https://github.com/systemjs/builder/commit/7c7ff83a83bfd7575474b3859909d029802b1675 which I believe resolves this.

guybedford commented 8 years ago

Released in 0.15.10.