toStringWithSourceMap behaves incorrectly if SourceNodes source is null or is an empty string

[Mingun]

When source set to null, mapping does not generated.
When source set to empty string there is an error during mapping calculation (not during SourceNode creation as expected if empty string is forbidden).

This code snippet demonstrates both problems: https://runkit.com/embed/47lduu6k2173

Source code of the snippet:

let SourceNode = require('source-map').SourceNode;

function test(source) {
  let node = new SourceNode(
    null,
    null,
    source,
    [
      'prefix',
      new SourceNode(1, 0, source, 'code'),
      'suffix',
    ]
  );
  let { code, map } = node.toStringWithSourceMap();
  console.log(map.toJSON(), code);
}
// {version: 3, sources: ["source"], names: [], mappings: "MAAA,I"}
test("source");// OK
// {version: 3, sources: [], names: [], mappings: ""}
test(null);    // Empty mapping
// Error: Invalid mapping: {"generated":{"line":1,"column":6},"source":"","original":{"line":1,"column":0},"name":null}
test("");      // Error

[hildjj]

I think we've gotten around this effectively with documentation, but perhaps I don't understand adequately. Is it still an issue going forward?

[Mingun]

At least, an error should be thrown in the SourceNode constructor instead of silent incorrect work, if it is desired behavior. At most, the problem should be fixed to allow any source

[hildjj]

(sorry, just noticed which repo this was on. I agree with Mingun that this is still an issue)

[ochameau]

At least, an error should be thrown in the SourceNode constructor instead of silent incorrect work, if it is desired behavior.
At most, the problem should be fixed to allow any source

It isn't crystal clear what would be the expectation when passing a null source.

We might convert any falsy source value in SourceNode constructor to become null.
So that we no longer throw in _validateMapping.
But then... what would be the expected mapping?

Today when passing null, we generate:

// {version: 3, sources: [], names: [], mappings: ""}
Would you expect the following instead?
// {version: 3, sources: [], names: [], mappings: "MAAA,I"}
or
// {version: 3, sources: [null], names: [], mappings: "MAAA,I"}

v3 specification isn't saying much about the expected content of sources array.

[Mingun]

From consistency point of view I think that

{version: 3, sources: [null], names: [], mappings: "MAAA,I"}

would the best solution for null, and

{version: 3, sources: [""], names: [], mappings: "MAAA,I"}

would the best solution for "".

[ochameau]

Would you have an example of a real usecase so that I can better understand what it means IRL to have null or empty named sources?

[Mingun]

No. I found this issue when I write tests for peggy in peggyjs/peggy#163. I don't think, that using null, undefined or empty string would be really used by someone, but I think it is better to provide consistent behavior in all cases.

As I said early, I will be satisfied with support in some form, that will provide consistent behavior, or throwing out an exception from SourceNode constructor is you decide that some inputs should not be used.