search

roman01la / html-to-react-components

Converts HTML pages into React components
https://roman01la.github.io/html-to-react-components/
MIT License
2.13k stars 136 forks source link

Conditionally exclude FS writing with env var #8

Closed jesstelford closed 8 years ago

jesstelford commented 8 years ago

Enables smaller browserify builds when combined with envify and uglifyify.

For example, with an invocation such as:

browserify -g uglifyify -t [ envify --NO_WRITE_FS ] index.js > out.js

Will conditionally exclude all the write to fs files (including the node fs module which doesn't work in the browser anyway.

note: The changeset is much better viewed with whitespace differences turned off: https://github.com/roman01la/html-to-react-components/pull/8/files?w=0 (due to indentation of a function)

jesstelford commented 8 years ago

I tried writing some tests, but I can't figure out how to get mocha to play nice with environment variables. It seems like no matter how many times I delete process.env.NO_WRITE_FS, it seems to stick around and everything else blows up :/

Any ideas? Some Google-based research didn't turn up much.

roman01la commented 8 years ago

Hi! I'm not sure, but I don't really like to use env variable here. Instead of using it you could just require html-to-react-components/lib/processor and use provided transformation function. Writing is done only in html-to-react-components/lib/index, you don't have to use it.

roman01la commented 8 years ago

@jesstelford Have you tried what I wrote in the last comment? Did it work for you?

jesstelford commented 8 years ago

Yeah, I definitely could reach into html-to-react-components/lib/processor, but is just feels a bit dirty to me; circumventing node's require algorithm.

It's probably not likely, but if you were to rearrange the files / refactor some functions internally, and do a minor or patch semver version bump, then any project which depends on html-to-react-components/lib/processor would stop working on the next install due to the default npm versioning using ^.

Is there a particular case in mind that you don't like environment variables for?

roman01la commented 8 years ago

I don't want to add specific features like this. But, I think it would be really good to have this tool working in the browser.

roman01la commented 8 years ago

Do you expect more PRs or browser support?