v2 proposal: Restructure index.js and remove deprecated code

[am11]

Since v2 is coming with breaking changes. Here is another batch, which I think will mitigate redundancies:

• Remove middleware (deprecated warning) export.
• Remove deprecatedRender and deprecatedRenderSync.
• Merge render and renderFile into render.
• Merge renderSync and renderFileSync into renderSync.
• Add options writeOutput.

The behavior goes like this:

• If user provides file in render( { file: '/path/to/file' } ), libsass will read the file, compile the code and returns back the code. We will pass the code (css: as string and map: as JS object literal) on to the caller, instead of writing it to the disk.
• If user provides data in render( { data: 'body { color: navy} ' } ), libsass will read the file, compile the code and returns back the code. Pass on code without writing to disk.
• If user provides both file and data in render( { file: 'c:/temp/example.scss', data: "body { color: white }" } ), libsass will give precedence to data and use file path in source-maps generation. In this case file will just make the parser more "context-aware". Again pass the code as is to the caller with no write to disk involved.
• If user provides writeOutput (which would be false by default) as in render( { writeOutput: true } ), we will write both CSS and source-maps to disk only then. (this shall address Emit source map as data (not file) #303)
• Same rules apply to *Sync variants.
• In case of CLI, writeOutput would be true by default and presence of --stdOut would be translated into writeOutput: false.

Our API will end up exposing only two methods:

{ render: [Function],
  renderSync: [Function] }

as opposed to the existing four:

{ render: [Function],
  renderSync: [Function],
  renderFile: [Function],
  middleware: [Function] }

which is confusing, because (1) middleware is obsolete, (2) there is no renderFileSync to complete the pair.

Future extension:

parseOnly: [Function]:

I'm not sure if libsass provide parse-only (lint mode) functionality, to check the code and reports back error without the compilation step involved (it is comparatively faster and will help the IDE's to validate syntax). However, if libssass' parser step is too tied up with compiler and there no such performance gain with or without compilation step involved; then the consumer of node-sass will just compile the code and grab the errors (pretend parse-only).

@andrew, @kevva, @mgreter thoughts?

[am11]

I have made the breaking changes in simplified_api via 35414a3.

• Now we only have render and renderSync functions.
• If data: option is provided along file:, data will take precedence.
• We will not write any file (unless of course from CLI usage). User would have control on how they want to store the data (css and source-map) in both render and renderSync cases.

Please let me know if you guys think this approach is flawed.

Next, I think we need to make custom importers (and custom functions in future) work with sync function as well. The problem is that, currently in our binding code we are not instantiating sass_context_wrapper like this in the Sync variants. I do not see it bringing any harm to use with Sync, although we might not be using most of the members defined in the struct sass_context_wrapper.

[jhnns]

Sounds good 👍

Concerning my suggestion on moving the stats-object into the result-object (and your comment):

Imho it feels kind of hacky to pass in a stats-object to the options that will be filled by node-sass. It was just a workaround because node-sass did return a string back then. It's probably a C-like approach (the object is like a void-pointer), but I would expect to get the stats back as result from the operation. Don't you agree?

[am11]

This makes sense. At this point its bit tricky to manage it while complying with DRY.

So here is what one way to deal with it:

sass.render({
  ..
  success: function(css, map, stats) {
    // map and stats both would carry source-maps. 
    // since the source-map's mappings size is directly proportional to that of input,
    // its anti-productive to have it twice.
  },
  error: function(error, status) {
    // error is the object literal (parsed JSON)
    // status is integer
  },
  ..
})

Therefore, it will probably be a good idea to strip source-maps off of stats (since you will always get both in the results anyway)

Then the matching sync variant would be:

var result = sass.renderSync({ ... });

// result.css returns css as string
// result.map returns map as string
// result.stats is an object literal without map (just meta info: how compilation transpired)
// if error, it prints out to stdOut as JSON without sparing any object return.

The point is, instead of making stats -- which is essentially extra info -- as a first-class citizen of result, we can just add it to result object as is (without map).

[jhnns]

Nice!

Just one thing: If renderSync() returns an object, I'd expect the success-callback to receive an object with the same structure:

sass.render({
  ..
  success: function(result) {
    // result.css returns css as string
    // result.map returns map as string
    ..
  }
  ..
})

[am11]

Well, since we are going to have number of breaking changes in v2 anyway, wouldn't hurt to incorporate this change. :)

[am11]

v2.0.0-beta is just released.