Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

empty source-map files incorrectly considered invalid #4

Open
stefanpenner opened this issue Oct 31, 2016 · 3 comments
Open

empty source-map files incorrectly considered invalid #4

stefanpenner opened this issue Oct 31, 2016 · 3 comments

Comments

@stefanpenner
Copy link
Collaborator

stefanpenner commented Oct 31, 2016

It seems that valid, but empty source-map files are considered invalid. sourcemaps.io appears to agree they are valid, just empty.

https://sourcemaps.io/validate?url=https%3A%2F%2Fgist.githubusercontent.com%2FTurbo87%2Fe39b740c872227698227d3fc7005ad36%2Fraw%2Fdc5f1ca29a8bb98030f63773cc2c4561026711ea%2Fnothing.js

related: broccolijs/broccoli-concat#51 (comment)

I can submit a PR you agree. If not, would love to here your thoughts on this.

@ben-ng
Copy link
Owner

ben-ng commented Oct 31, 2016

I vaguely remember making empty sourcemaps fail because they're usually a sign of programmer error... similar to how compilers will warn if you have dead code, and how git doesn't let you make empty commits (without the --allow-empty option).

Rather than make them valid, maybe we should have the error be something like "Warning: The sourcemap appears to be empty. You can set allowEmpty=true to suppress this warning." and then add the option to the validator.

What are your thoughts?

@stefanpenner
Copy link
Collaborator Author

I vaguely remember making empty sourcemaps fail because they're usually a sign of programmer error... similar to how compilers will warn if you have dead code, and how git doesn't let you make empty commits (without the --allow-empty option).

The assertion "is this the right source-map" is different from "is this a valid source-map". Today this module only tests "is this a valid source-map", the exception to this is the assumption "empty map" is invalid.

Now, I do see the value in this case. So let me propose:

  1. by default, continue to disallow empty maps
  2. the assertion for empty maps become as follows "did not expect and empty map, if this is expected please provide allowBlank: true" or something..

@ben-ng
Copy link
Owner

ben-ng commented Nov 1, 2016

I think the proposal is a good one, and is consistent with the behavior of similar tools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants