Comments (16)
Can't we make it validate sourceMappingURL string only at the beginning of the line as it's supposed to be? Please see referenced issues, coming from AVA and then nyc. Thanks.
from convert-source-map.
@zanona It's in v1.5.1
as 46a6c17
from convert-source-map.
@zanona Yeah, that would be best. It's not mentioned in the spec that //# sourceMappingURL=
must be at the beginning of the line, but I haven't seen any compilers not do that. And we can still allow whitespace in front, I guess.
Any thoughts, @thlorenz?
from convert-source-map.
That'd be a breaking change, could we at least do something like where it can only have whitespaces in between the beginning of the line and the comment?
That'd allow things like ' //# sourceMappingUrl='
.
from convert-source-map.
Realizing that this wouldn't fix the coffee script problem :(
How about a option we pass, i.e. { strict: true }
that will do what @zanona suggests?
That way we wouldn't break any existing libs as we'd set strict: false
by default.
from convert-source-map.
@thlorenz, the only problem is that currently there are quite a few packages depending on convert-source-map
. I for one, came from ava and then nyc and here 😜.
So this would require all these packages to change their codebase in order to upgrade which I am not sure they are willing to do.
To be honest, I see this as a breaking-change, yes, but it's a bug fix as well, so strict
in this case would be the right way of handling that which should always default to true
and not false
.
I really don't want to assume, but my initial thought is that most users are indeed only validating //# sourceMappingUrl=
for its real purpose (converting source maps) and not using that string within code, otherwise they would probably have reported this as a bug, because it would not be expected.
What do you guys reckon?
from convert-source-map.
I believe your assumption is correct, but still some modules may break with strict: true
as default.
To avoid that we'd make it false
and then the coffee-script people can bug the authors of other libs to add the strict: true
flag.
I understand that's more work, but I'd favor that over breaking a bunch of modules.
It can have pretty far reaching effects since tons of modules use convert-source-map directly or indirectly. So we have some responsibility here.
However if enough collaborators think otherwise I'm open to follow your suggestion anyways @zanona .
Let's see if others chime in ....
from convert-source-map.
@thlorenz, I completely understand and I believe we have good options.
next patch
Setting strict: true
in order to get the intended behaviour (default is: false)
next major release
Adjust default strict
option value to true
since is really the expected behaviour
@novemberborn would you be happy with that so it's adjusted on nyc?
Perhaps you could let us know your thoughts on this as well?
refs: avajs/ava#1595, istanbuljs/nyc#726
from convert-source-map.
I disagree with adding a strict
option in the next patch. Just fix the template string case with #61, then add the breaking change in next major.
Then library authors don't need to opt-in to get the correct behavior for template strings, which is the real issue right now.
from convert-source-map.
It's worth noting that commentRegex
has a leading ^\s*
.
Is there a reason mapFileCommentRegex
doesn't already have that, or just a slip up?
Since inline source maps already have the behavior that @zanona is suggesting, perhaps it makes sense to patch the same behavior into mapFileCommentRegex
.
from convert-source-map.
commentRegex has ^\s*
so it would still break the coffee-script case for template strings.
I have no problem adapting the mapFileCommentRegex to behave like that, but I don't want to remove the optional whitespace chars since that could break people.
Then library authors don't need to opt-in to get the correct behavior
Correct or not correct there might be source maps out there that look like the below
some code here
//# sourceMap...
(i.e. they have a space at the beginning of the source map line).
I'd like to avoid breaking these cases.
from convert-source-map.
@thlorenz Those cases would not break by merging #61 (what I suggested we do). If we patch in a strict
option (that defaults to false), the template string case won't be fixed without effort by the library author.
commentRegex has ^\s* so it would still break the coffee-script case for template strings.
I think you misunderstood me. I was pointing out that commentRegex
already skips comments with non-spaces in front. So the current behavior for inline source maps is already different from sourceMappingURL
comments. I still think patching #61 is the best option, and adding ^\s*
to mapFileCommentRegex
should come in a major release.
from convert-source-map.
Ah, ok I get it now. Yeah mapFileCommentRegex are less forgiving (they were added much later than the souceMappingURL).
So I wouldn't change anything about that behavior as they are already strict and things work like that.
However let's keep the commentRegEx non-strict by default (is my suggestion).
We can merge #61 since that seems fine to me and think about the strict
flag out side of that PR.
from convert-source-map.
If I understand correctly #61 will only fix one liner template strings, but not things like
`
my string with some info about how to use
//#sourceMapComment
`
at any rate there seems to be no simple fix either way for the below case:
`
my string with some info about how to use
//#sourceMapComment
`
from convert-source-map.
Yeah, I don't think RegExp
can do look-behind on line before first matched line. I highly doubt anyone is doing such a thing, and it would be ill-advised to do so. Let's merge #61 and publish v1.5.1 asap. 😄
from convert-source-map.
Guys, do we need any other info or are relying on something else to make this happen?
from convert-source-map.
Related Issues (20)
- SourceMappingURL causing failure on Safari HOT 10
- Fail toComment with included content. HOT 1
- Tests fail due to charset issue HOT 8
- Update inline-source-map to 0.6 HOT 1
- What is the correct way to remove a property? HOT 2
- VM6510:1 Uncaught SyntaxError: Unexpected token u HOT 1
- Error when running angular2-seed build process?? HOT 1
- Support non-base64 encoded json? HOT 4
- Unclosed block error in map-file-comment-double-slash.css HOT 1
- Rogue sourceMappingURL comment trips up devtools HOT 1
- source-map file name with query parameter, causes source-map not found HOT 6
- Reduce package size HOT 10
- sourceMappingURL with non-base64 data URI not supported. HOT 9
- sourceMappingURL inside string literal
- Module not found: Error: Can't resolve 'fs'
- Braking typescript/nyc coverage report HOT 1
- Extract string methods, browser support
- Safe to remove safe-buffer HOT 1
- memory consumption for 1.9.0 HOT 5
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from convert-source-map.