Don't define transform as global

[spion]

Global transforms run after all other transforms, which means that all other transforms will try to process the CSS file.

This is bad for many transforms that are like e.g. brfs, which don't check whether the file is valid JS before proceeding to try and parse it (with acorn)

This patch makes https://github.com/css-modules/browserify-demo/ work

[joshwnj]

Thanks for the PR @spion . I added the global flag as it seemed like that was necessary to allow requireing css from node_modules. You can read more about that and see the test case here: #24

Perhaps there's a different way to go about it?

Regarding brfs I'm waiting for browserify/brfs#51 to be accepted which will allow us to avoid the problem you mentioned. I guess in the meantime I'll switch browserify-demo to use my fork of brfs.

[spion]

@joshwnj I see - that should work.

I wonder if it would be possible to support pre-compiled CSS modules though. AFAIK one of the assumptions for npm modules is that they're distributed as pre-compiled (thats why browserify transforms aren't normally run on them).

[serapath]

I'm not sure if I understand but the imported package can have a browser field in package.json to specify transforms - see https://github.com/substack/node-browserify#browser-field

[joshwnj]

@spion & @serapath pre-compiling is a good idea and it would be great to explore this space. But since CSS Modules is not tied to browserify in any way I'm not sure we would want to require authors to include a browserify transform in their package.json.

Using a global transform seemed to be the quick easy way to do this, but I'm aware there are maybe some unwanted side-effects :) If you take a look at the unit tests (and specifically, the one which fails with this PR) you'll see what I'm aiming for. Maybe there's a better way to reach that without using a global transform?

[spion]

@joshwnj it would not necessarily be tied to browserify. The way I imagine it, there would be:

1. an intermediate step (the transform) which would mainly output JS.

   This could be as simple as adding a $$__css_module_content export:

   module.exports.$$__css_module_content = loader.finalSource

   The CSS extension would be kept (this should not be a problem as the transform would be non-global)

   Another alternative would be producing intermediate CSS files. The JS file would again keep the CSS extension, while the actual content would be in a different file e.g. file.css.precompiled

2. a bundling step (the plugin) which would look for the intermediate modules and produce a final CSS file by bundling them all

   Would look for module.exports.$$__css_module_content, and also remove it from the module's export list. (Alternatively, it would look for css files which have a corresponding ".precompiled" file in the same directory)

Step (2) would only really be necessary when bundling the final app, while step (1) would be done by module authors when publishing their module (e.g. on npm)

The way this will work in other bundlers (webpack/jspm) would be by detection: if the CSS file is a JS file, its considered precompiled; otherwise, it hasn't yet been compiled. (Alternatively, the existence of a corresponding .precompiled file could be checked)

This is all a rough and sketchy idea which doesn't necessarily blend ideally into css-modules, but its the best I've got :) Given that browserify is built on the premise that a transform's output is always JS, I'm not sure if its possible to come up with something that would work elegantly across browserify, webpack and jspm...

[nchase]

hi folks – I'm just digging into this. looks like the folderify module has this same issue.

[joshwnj]

Hi @spion, closing this now as in 0.16.0 (#64) the transform is not global unless you opt into that: https://github.com/css-modules/css-modulesify/blob/master/index.js#L96

Please let me know if it's still an issue