marksmccann / node-sass-extra

A drop-in replacement for node-sass' Node API that adds support for globs, promises and more.
MIT License
2 stars 1 forks source link

Implement file output for compiled Sass/SCSS #6

Closed phillipluther closed 5 years ago

phillipluther commented 5 years ago

Establish rules and functionality for writing files to disk.

If a single file is specified as the output, do compiled Sass/SCSS files get concatenated? Or do will we only support outputting a 1-to-1 mapping of source files into a specified output directory.

So in the case of:

src/
    my-file.scss
    my-other-file.scss

Leads to ...

> node-sass-extra src/**/*.scss -o dist

dist/
    my-file.css
    my-other-file.css

Would ...

> node-sass-extra src/**/*.scss -o dist/compiled.css

dist/compiled.css
// compiled contents of both my-file.scss and my-other-file.scss

This effort will also implement iterators to use when creating dynamic output files.

import sass from 'node-sass-extra';
...
sass('src/**/*.scss', {
    outFile: srcFile => srcFile.replace(/\.s[ca]ss/, '.css')
});
marksmccann commented 5 years ago

Would ...

> node-sass-extra src/**/*.scss

src/
    my-file.css
    my-other-file.css
// compiles files into current directory
marksmccann commented 5 years ago

RE: Concatenating multiple src files into a single output file.

Initially, I thought, "no, that's weird". But, on second thought, I am actually quite charmed by the idea. You (the developer) is explicitly saying, "I want all of these files to compile into this one file." I like the idea. It also could eliminate the need for a manifest file like we had in the design language (main.scss).

marksmccann commented 5 years ago

RE: Throwing multiple src files into a single output directory

This one seems like a no brainer, very handy. Only question is would the existing path structure remain, I would assume it would.

So given:

src/
    my-dir/
        my-file.scss
    my-other-dir/
        my-other-file.scss

This...

> node-sass-extra src/**/*.scss -o dist

... would compile to this ...

dist/
    my-dir/
        my-file.css
    my-other-dir/
        my-other-file.css

... or this?

dist/
    my-file.css
    my-other-file.css

I am torn on this one. My gut says, it should be the former (where the folder structure is preserved) because it is more "pure". The later would also be easier to accomplish via the dynamic iterator feature, but it might also be nice to be able to perform this task from the command line. We could create a new argument to flatten the output file?

> node-sass-extra src/**/*.scss -o dist --flatten
marksmccann commented 5 years ago

RE: Dynamic Iterator for the output file path

I think this is also a feature we should definitely include, but I have a few questions.

  1. Instead of passing in the srcFile argument to the iterator, what if we passed in the projected output file? I mean, why force the consumer to change the file extension to .css every time when we could do it for them.

so instead of...

sass('src/**/*.scss', {
    outFile: srcFile => {
        srcFile; // src/my-file.scss
    }
});

It would be ...

sass('src/**/*.scss', {
    outFile: srcFile => {
        srcFile; // src/my-file.css
    }
});

We could also potentially take advantage of other features, to make it even easier?

sass('src/**/*.scss', {
    outDir: 'dist',
    outFile: srcFile => {
        srcFile; // dist/my-file.css
    }
});

I dunno, maybe this convolutes the feature, just wanted to include the idea.

  1. So the primary reason I wanted to add the config file to the CLI was to support this feature from the command line. But, it would be even cooler if we could support this feature without even a config file. Not sure if that is possible or what it would look like. Maybe there is a regex or interpolation command line syntax we could use, I dunno, just wanted to ask the question.
marksmccann commented 5 years ago

Another idea could be to add additional props to manipulate the output file. Unfortunately, this would complicated the API, but there could be some really useful features - and it would allow more use cases to use the component from the command-line only.

sass('src/**/*.scss', {
    outDir: 'dist',
    fileExt: '.min.css', // default is .css
    ...
});
phillipluther commented 5 years ago

RE: compile into the current directory if no --output (or ultimately named param) is specified

I don't know. Should it? I'm leaning towards making --output explicit; if you want it in the current directory, say so.

node-sass-extra src/**/*scss -o .

... since the original node-sass package doesn't output anything. There may still be use cases for just getting the compiled CSS as a string. File output is an opt-in, but super-easy/accessible, feature.

RE: concatenating

That's how I've got it set up on the current WIP branch; if you pass an array of files, it'll compile them and concat them together.

sass({
    file: ['file1.scss', 'file2.scss'],
    output: 'bundle.css'
});
// compiles file1, compiles file2, concats file2 to file1, writes content to bundle.css

RE: preserving directory hierarchy

The directory structure is preserved on the current working branch, but badly. It actually carries the whole source directory into the dest, which I don't think is right.

sass({
    file: 'src/source-file.scss',
    output: 'dest'
});
// dest/src/source-file.css ... blech

I like the --flatten option a lot; we had something similar on the previous shall-not-be-named proprietary compiler, didn't we? I agree with you; that seems like a good measure - intelligently preserve structure unless told not to.

RE: dynamic iterator

The example in the issue is an awful one (changing scss to css); the WIP branch does that automatically, as you suggested. I'll clarify it.

I like the line of thought about providing the source/dest path, though. What if we did both?

sass({
    file: 'src/nested/my-file.scss',
    output: 'dest',
    outputFile: (sourcePath, destPath) => {
        console.log(sourcePath); // src/nested/my-file.scss
        console.log(destPath); // dest/nested/my-file.css
    }
});

That way you have a record of what the file was and what it will be; you can do any number of transforms based on the rest of the config options you've provided without having to duplicate anything.

RE: include additional props (like file extension)

That's a cool idea, not just for .min.css but for creating hashes or something. I vote we create a new issue for that one, but not include it as an initial release feature.

marksmccann commented 5 years ago

RE: compile into the current directory if no --output (or ultimately named param) is specified

Agreed, output should be explicit. Having it compile into the current directory if none is provided seems like an unexpected behavior to me. I'd say, for the sake of matching the current API, let's leave it out. Apparently, in the CLI, if no output is specified, the css is consoled out.

RE: preserving directory hierarchy

Apparently, the node-sass CLI automatically flattens the path when an output dir is provided. Perhaps there is an option to preserve the directory structure? OR maybe this is when we rely on the callback function and allow the consumer to just handle it themselves? I'm thinking the later.

RE: dynamic iterator

Bear with me through the following brain dump ...

I'm a little confused by your example. You have two file props. I assume the later is meant to be outputFile? In either case, since the CLI automatically flattens the output file when a directory is provided, your example should actually look like the following, right? –

sass({
    file: 'src/nested/my-file.scss',
    output: 'dest',
    outFile: (sourcePath, destPath) => {
        console.log(sourcePath); // src/nested/my-file.scss
        console.log(destPath); // dest/my-file.css
    }
});

So to be clear we are proposing –

But...

What happens when you want, for example, to use the output prop to generate source maps but don't want to write to disk? Should we create an explicit write: true prop that will write the given dest path (from output and/or outFile) only when provided? What if we abandoned output altogether and relied on the outFile to handle that? What if output receives the callback and outFile just gets defaulted to the response of output?

Here are some examples of how I'm thinking this could work –

// writes all files (flattened) to `dest/`
sass({
    file: '**/*.scss',
    output: 'dest'
});
// writes all files to generated path
sass({
    file: '**/*.scss',
    output: (srcPath) => {
        // generate and return desired path
    }
});
// generates the desired path but does NOT write to disk
sass({
    file: '**/*.scss',
    sourceMap: true,
    outFile: (srcPath) => {
        // generate and return desired path
    }
});
// `outFile` will be defaulted to path created by `output`
sass({
    file: '**/*.scss',
    sourceMap: true
    output: (srcPath) => {
        // generate and return desired path
    }
});
// If both were provided, `output` would override `outFile`?
sass({
    file: '**/*.scss',
    sourceMap: true,
    output: 'dest',
    outFile: (srcPath) => {
        // this would be ignored
    }
});

The above examples would accomplish the following —

  1. The new output prop will match the behavior of the CLI (when provided you are explicitly telling the tool to write to disk)
  2. The callbacks of both output and outFile would enable support for globs
  3. When output is provided, you will not need outFile
  4. None of this features will be a breaking change to the API
marksmccann commented 5 years ago

RE: dynamic iterator

lerna has this neat feature for dynamically creating the commit message. They use placeholders in a string and replace them with the version number. For example:

lerna version -m "chore(release): publish %v"
# commit message = "chore(release): publish 1.0.0"

What if we did something similar to allow users to dynamically create the output file. This would enabled most users to use the feature directly from the command line. It would also negate the need for additional props like fileExt.

node-sass-extra src/**/*.scss -o dist/%n.min.css

dist/
    my-file.min.css
    my-other-file.min.css

We could possibly use Path's Path Object as the variables to replace.

So, in the case of:

/src/path/to/my-file.scss

These could be the options:

%d (dir) -> src/path/to
%r (root) -> /
%b (base) -> my-file.scss
%n (name) -> my-file
%e (ext) -> .scss

We could also just make up our own, I mean how useful would ext or base be when the file extensions is always going to be .css instead of .scss?

This feature would also enable the preservation of the file tree (since we may default the behavior to flattening the files)

node-sass-extra src/**/*.scss -o dist/%d/%b.css
phillipluther commented 5 years ago

I like your Lerna example a lot - a standardized set of interpolations for creating output names. If we're thinking of this thing as a command line-first tool, that seems incredibly natural. I like how it doesn't pollute the API, too.

The alternative in a Node env would be to use the callbacks/transforms to change your filename, right? When you have access to logic and scripting to do something fancy, use it; when you just wanna create a pattern to swap some standard parts of a path on the CLI, use that.

Gut feeling, I also agree that %b and %e would not be useful as a command line things ... at least for initial release. We can assess that later if clamor arises. If we just had %d, %r, and %n from your example above you could cover the vast majority of needs - namely change file extensions or specify directory structure relative to the project or the file location.

phillipluther commented 5 years ago

That was totally a typo in my comment here: https://github.com/marksmccann/node-sass-extra/issues/6#issuecomment-513435332

... which caused your confusion here https://github.com/marksmccann/node-sass-extra/issues/6#issuecomment-513468955

... regarding two file props. I updated the example snippet.

After mulling, I hate the idea of altering behavior of outFile ... node-sass has that established, and I think we should leave it alone. I like your examples of tapping into output a lot more. And in fact, I think I was thinking of it even more simply - if output is a file (containing an extension), everything in file is concatenated and written there. If output is a directory, everything in file is kept separate.

N inputs to N outputs, or to a single output. If you want to create different files or split things up, make subsequent sass calls.

sass({
    file: ['file1.scss', 'file2.scss',
    output: 'explicit/path/or/function/that/returns/a/path.css'
});
// concats and outputs to `output`

sass({
    file: ['file1.scss', 'file2.scss'],
    output: 'dir/or/function/that/returns/a/dir'
});
// file1.css and file2.css written to dir/or/function/that/returns/a/dir
marksmccann commented 5 years ago

... regarding two file props.

Agreed, we should not change how outFile works, it should not write to disk and it should primarily continue to be used for conjunction with source maps. However, adding some support for at least dir/or/function/that/returns/a/dir seems necessary. Otherwise, the prop (and source maps) becomes worthless in tandem with our multi-file compilation support.

Another concern I have with your proposal here is that I fear it is not flexible enough. What about the very likely use case someone wants to preserve the input file's file tree in the output. Are you suggesting that in that scenario the developer would have to re-wrap our component again and make subsequent calls on their own? That seems like a waste. Why would they want to use our component at all then? At the very least, making both output and outFile functions that return a dir or a file seems like a good solution that would cover any use case and would not be a breaking change.

phillipluther commented 5 years ago

Closing; added to initial-release branch by https://github.com/marksmccann/node-sass-extra/commit/86c736869dfafc5d22455da2b04c1b5be1f820a8