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

one-to-many file support #38

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

chriseppstein
Copy link

This is currently published as a fork, but if the direction here is something folks like, I will be happy to re-work this PR into something mergable. For now, I'd like to get feedback. Specifically, I find that the targetExtension property, probably doesn't make as much sense now but I'm not sure how to maintain backwards compat otherwise.

From the original README:

Q: Can this help with compilers that are almost 1:1, like a minifier that takes
-a .js and .js.map file and outputs a .js and .js.map file?

A: I don't know yet how to implement this and still have the API look beautiful. We also have to make sure that caching works correctly, as we have to invalidate if either the .js or the .js.map file changes.

I'm not sure if this API is considered "beautiful" (such is in the eye of the beholder) but it is identical to the original API until you need to add a second output file at which point you just need to accept a new argument and call a callback with the string and the relative filename. This works with caching.

@chriseppstein
Copy link
Author

@stefanpenner @joliss ping

@@ -73,7 +85,7 @@ describe('Filter', function() {
function write(relativePath, contents, encoding) {
encoding = encoding === void 0 ? 'utf8' : encoding;
mkdirp.sync(path.dirname(relativePath));
fs.writeFileSync(relativePath, contents, {
fs.writKeepOriginalFiltereFileSync(relativePath, contents, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what is going on here.

@chadhietala
Copy link
Contributor

@chriseppstein I like this approach as opposed to a hook. Adding a hook a dev implements is more in the vein of Broccoli APIs, however I think you would loose the flexibility that this approach affords.

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.

3 participants