Open reubenrybnik opened 3 years ago
I believe you should add this as a PR, even as a draft. That way it can be reviewed.
I believe through2
is no longer required, as it can be replaced with Node's simplified stream API constructor.
new Transform({
objectMode: true,
transform() { /*... */ }
})
I feel in this case, the PR should remove Vinyl
and stream the chunks (RollupOutput["output"]
) instead. This way a wrapper can be made to implement transforming each chunk to a vinyl instance. To activate it, set a flag in the options to true (options.objectMode).
Related links:
I've been sculpting a gulp plugin here: https://github.com/waynevanson/rollup-stream-gulp
I made an issue regarding a gulp plugin here https://github.com/rollup/rollup/issues/4136
Thanks for the feedback @waynevanson, and it's nice to see your interest in helping Rollup to work better in a Gulp pipeline in rollup/rollup#4137! I like the approach you've suggested to avoid needing to add optional dependencies to this plugin by using native object streams and converting to Vinyl further down the stream similar to how vinyl-source-stream
currently works; I'm not familiar with optional dependencies and how best to convey the circumstances under which they should be installed so not needing to include them will be nice.
Providing the ability to leave comments on specific lines probably would have been nice even for code that I knew was nowhere near the finish line, and I'll keep that in mind in the future. I'll probably start a new branch for these changes, though, since I don't think any of what I've currently written will survive aside from maybe the options modifications.
I'm also happy to step back if you'd prefer to make this change yourself as part of your larger project @waynevanson; the modified version that I provided a diff for works for my needs, so I was just suggesting this in the interest of possibly contributing back to the community if such a contribution is desirable.
This isn't a very frequently maintained package. We'd be happy to review a PR from the community to resolve the issue.
Feature Use Case
I'm using Rollup to bundle node dependencies specifically (not an entire app), and to do that efficiently it is useful to define multiple named entry points in a single config. With this approach, dependencies of dependencies which are sometimes shared between dependencies and thus can benefit from having multiple entry points that are evaluated together are also efficiently output as separate chunks as necessary via code splitting. The current examples, however, don't appear to support this output well since with them it seems like I can only have one file output and I need to know what it's called. To be able to do this efficiently, I believe that I need to have a mode for this plugin in which it wraps the files in Vinyl streams itself to provide multiple file outputs and preserve the names of those files.
The reason I'm not using gulp-rollup for this is that it seems to imply that I'd need to load every file that Rollup might need in
gulp.src
, which seems inefficient, particularly because I'd ideally also want to avoid pulling in dev dependencies. I could maybe avoid providing any source files at all using this option, but that seems to be discouraged 🤣The
rollup.config.js
that I'm currently using for this is below:Feature Proposal
It would be nice if this plugin offered an option to provide multiple chunks from Rollup as separate outputs in a format that's easy to consume since it seems like such output isn't available in the current version. I'm interested in potentially contributing a change that accomplishes this goal back to the main repository if this is a feature that maintainers are interested in adding or possibly documentation that shows how to use this plugin to accomplish such a goal if this feature already exists and I missed it.
I've put together a proposed implementation at https://github.com/rollup/stream/compare/master...reubenrybnik:output-vinyl-stream that is comprised of an optional flag which when specified causes this plugin's output to be changed to a Vinyl object stream to which the separate named files provided by Rollup are written. I'm currently using this implementation in a private commercial repository. This works for my use case but may not be the best implementation of this feature from maintainers' perspective, and I'm happy to discuss potential alternatives in that case.
If this approach is accepted, I believe my current proposed implementation of adding an option to switch to this output mode and making the Vinyl dependencies optional would make this a non-breaking change. However, I'm interested in knowing maintainers' views on whether it might be better to move away from the current pattern of using
vinyl-source-stream
to convert the output to a single Vinyl item. In particular, I'm not sure whether there are any use cases in which this output is not immediately converted to Vinyl or whether maintainers are interested in supporting such use cases. I'm also interested in views on whether it would be better to switch to this mode using an option as in my current implementation or whether it might be better to provide a separate function property off of the main export instead (i.e. something likewebpackStream.vinyl(config)
instead ofwebpackStream(config, { outputVinyl: true })
). I'd also welcome any other feedback you'd like to provide on this approach.Thanks in advance for your consideration of this request!