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

Add regex to check for .css files #31

Closed
wants to merge 1 commit into from
Closed

Add regex to check for .css files #31

wants to merge 1 commit into from

Conversation

MylesBorins
Copy link

Currently brfs will attempt to process css files if they are included from the node_modules folder. This check will assure that brfs will never attempt to process .css files if they somehow get processed by the transform.

I am not 100% sure this is the most elegant solution to the problem due to the nature of the edge case necessitating this bug fix, attempting to require a .css file from a submodule

require('beep/boop.css');

In this particular case the transform is not run on this require statement, due to it not being in a submodule. If the transform is run globally instead (-g), brfs attempts to process the css file on the first pass.

My reservation is that I'm not completely sure that brfs should care about specific file types, although since .json is already explicitly ignored perhaps my intuition is wrong. My main concern is that the number of potential dot files is infinite. Would it perhaps be better to make a white list rather than a black list?

Currently brfs will attempt to process css files if they are included from the node_modules folder. This check will assure that brfs will never attempt to process .css files if they somehow get processed by the transform.

I am not 100% sure this is the most elegant solution to the problem due to the nature of the edge case necessitating this bug fix, attempting to require a .css file from a submodule

```
require('beep/boop.css');
```

In this particular case the transform is not run on this require statement, due to it not being in a submodule.  If the transform is run globally instead (-g), brfs attempts to process the css file on the first pass.

My reservation is that I'm not completely sure that brfs should care about specific file types, although since .json is already explicitly ignored perhaps my intuition is wrong.  My main concern is that the number of potential dot files is infinite.  Would it perhaps be better to make a white list rather than a black list?
@ghost
Copy link

ghost commented Aug 7, 2014

You shouldn't run brfs in global transform mode. The problem is there is more than just css, there are many many extensions and I don't want to spend time policing special cases. Instead of global transforms, add a package.json to each package that needs a transform locally.

@ghost ghost closed this Aug 7, 2014
@MylesBorins
Copy link
Author

This edge case was not caused by brfs being run globally, but another transform being run globally causing brfs to create a false positive. Perhaps you can show me on irc what is going wrong.

On Aug 7, 2014, at 12:55 AM, James Halliday notifications@github.com wrote:

You shouldn't run brfs in global transform mode. The problem is there is more than just css, there are many many extensions and I don't want to spend time policing special cases. Instead of global transforms, add a package.json to each package that needs a transform locally.


Reply to this email directly or view it on GitHub.

@MylesBorins
Copy link
Author

fwiw I found a way around having to catch this edge case. I simple symlinked the file from node_modules to the static directory my app is server. It isn't elegant but it works

FamousArchives/browserify-seed@b469d61

This pull request was closed.
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 this pull request may close these issues.

1 participant