Open timhall opened 6 years ago
Oo, cool! 🎉
I had a similar idea on this --- except, for me, it was going to be part of #286 because sourcemap
ping is a utility feature.
By that same token, I definitely don't want it touching core
at all. The reason being that Taskr is a task-runner... not a build tool. It can be used as one, but that's not its primary function.
I can work with you in finalizing this, especially when it comes to extracting a @utils
package.
But the general idea is that any *.map
file is just a file. Adding or altering the map is exactly the same process as adding or altering any other file. The actual sourceMap
logic can be handled inside the package (or even offloaded to another module, like you have), but once that logic has been completed, it'll push its changes to the current task._.files
array.
That way, when it comes to target()
the output, the map's final-form is written to the destination file.
A grand total of 0 changes need to be made in core for that to happen. Nor any added dependencies!
Does that sound fair/reasonable to you?
As mentioned, happy to collaborate with you on this. FWIW I do think this is needed -- would have made things easier in writing a bunch of plugins 😆.
Yeah, I figured this would go hand-in-hand with splitting off utils (had to copy read
in temporarily until that's done).
I would argue that source
and target
are file-specific and a source map is file metadata that makes sense to live with the file, but I can totally understand reducing API surface area in core.
Should be fairly straightforward to make this only at the plugin level, with a standardized sourcemaps
option that is either true
for inline or a path for external. applySourceMap
would load and merge into an existing source map from inspecting file.data
. Some potential tricky issues are filtering previous external maps from subsequent plugins (should only use inline until the last step if you want external) and ensuring the user enables sourcemaps for each step, but they're docs-type fixes.
Right, source
and target
(potentially) mark the beginnings & ends of pipelines. What happens in between means nothing to them. A sourceMap
is as irrelevant to them as a styles.css
or a 10MB archive. What happens in between those two methods means nothing to the methods themselves.
To your point tho, I did consider extract source
and target
into "plugins" (leaving start
, serial
, parallel
, and plugin
as the only API) for philosophy consistency, but I figured that that may have felt too modular/segmented to the existing users, since 99% of use cases (even my own) rely on source
.
This might still happen down the road.
I see plugin authors doing this:
const babel = require('babel');
const utils = require('@taskr/utils');
module.exports = function (task) {
task.plugin('babel', {}, function * (file, opts) {
// ...
// both are one of: `true` | `both` | `inline`
opts.sourceMap = opts.sourceMap || file.sourceMap;
const isType = opts.sourceMap;
const output = babel.transform(file.data, opts);
if (opts.sourceMap && output.map) {
utils.sourcemaps.call(task, file, output.map, isType);
}
// every call will update matching `file.base`.map
// or update internal `file.data` content
});
}
Names and such can be changed, of course. But the idea is that utils.sourcemaps
handles all the writing & updating logic... just like any other plugin would for its own API.
I realize that muddies a "util" vs a "plugin". To remedy, I think the logic should be mostly within the utils.sourcemap
& then the would-be plugin imports & wraps that with an official name & some methods.
Thinking out loud here, sorry for ramble. 😆
This PR adds source map init, write, and apply for use in tasks and plugins. The approach is very similar to Gulp 4 (in fact most of the code is based on gulp-sourcemaps). Important improvements include:
Usage
Build-in support:
Custom usage:
Plugin usage:
Discussion
1. Should this be included by default with the
taskr
packageI think building gulp-sourcemaps into gulp 4 was well received and would be a nice addition to taskr. Currently, most of the plugins have source map support currently, which is awesome, but they only support merging if the tool supports it (which is rare) and some don't allow control between inline and external source maps. This adds 4 direct dependencies, but they have no sub-dependencies, so in total it's fairly reasonable at 80k.
Alternative: It could potentially be an optional dependency that needs to be explicitly installed, but it will most likely be installed by plugins needing
applySourceMap
that in the end it may not lead to savings for the user to make it optional.2. Should source maps be written by default
Currently, unless source map writing is explicitly disabled with
.target('...', { sourcemaps: false })
, if thesourceMap
property is found on a file, it is written as an inline source map. ThesourceMap
property should only be set if source maps are enabled in.source
orinitSourceMaps
, so it's reasonable to assume it is an intelligent default to write them.I've added support to
@taskr/babel
, wanted to see what you think before I went any further. Thanks!