browserify / watchify

watch mode for browserify builds
Other
1.79k stars 181 forks source link

Adding transforms via browserify's packageFilter breaks watchify #226

Closed dgbeck closed 9 years ago

dgbeck commented 9 years ago

Driving me nuts. The following gist is a bare bones module that reproduces the issue. To reproduce, download

https://gist.github.com/dgbeck/87d7e7efd4f8881b2ed8

Go to the directly and run npm install. Then run

> node build.js

Then change the template.tpl file while watchify is still running. In this example, the lowercase letter "i" was added before "This". The following will be output to the console:

Error:
/Users/david_beck/Downloads/gist87d7e7efd4f8881b2ed8-cd05643585f99619a0c90a23bb493dc69ee6005a/template.tpl:2
iThis is a <b>html</b> template.
      ^
ParseError: Unexpected token

It appears the transform added to the package object by the packageFilter is not being considered when bundle is called the second time (although the packageFilter function is being executed).

Hoping there is an easy fix for this issue. Hurts to rebuild every time we make a change to a template.

zertosh commented 9 years ago

I just published the fix in watchify@3.2.2. However, your use of packageFilter is really strange. Why not add the transform to the package.json file?

dgbeck commented 9 years ago

Right on! Thank you, @zertosh !

We use packageFilter in order to safely apply "global transforms" only to specific directories via cartero's appTransforms option. This will be a big help.

Thank you again!

zertosh commented 9 years ago

np. oh what an interesting use case. can you tell me a little more? i never considered this as a way to do that, but yeah, i can see how it works.

dgbeck commented 9 years ago

Hi @zertosh ,

k not sure which part you are curious about, but can just ramble a bit.

The way we architect our applications (using cartero as a build tool), we put client side assets in the views folder, and each view is actually a little sudo-module that contains a package.json. I say "sudo-module", because each view is clearly a part of the application, and is not intended to be used as a stand alone module. They share styles, scss mixins, templating conventions etc, with the application. Therefore the transforms that are applied to these sudo-modules are semantically application level. Also, there are other modules that we keep in a "lib" folder that is symlinked from node_modules that are shared between multiple pages or componentes of the application. (So we can do require( 'lib/file-uploader'), for example.) These, too, are sudo-modules, in that they are tied to the application. To drive home that point, the scss tranform that is applied to these modules needs to be passed a "includeDir" option that points to an application mixins directory, so that we can use application mixins from these sudo-modules.

In any case, browserify global transforms can work as application level transforms, but as @substack has expressed many times, global transforms are dangerous and should be used sparingly. We implemented the more explicit application level transform to be safer and more closely the semantics.

Does that answer your question? Thanks again for your help resolving this issue!

zertosh commented 9 years ago

thanks for the explanation @dgbeck!