feat!: Support URI encoded source maps #75
Conversation
|
I like this in general, but we gotta be super careful here. Every time we change the regex it may break in certain scenarios. If the RFC regex is more correct we should make it the default going forward and just provide the old one for people upgrading that run into issues. Did anyone actually complain ever that our existing regex doesn't match correctly? |
Jdalton complained in #59 |
|
Since we are going to release this as a major. Can we remove the original |
* Export a new regex compliant with RFC 2397. * Recognise comments without base64 encoding. * Add methods to convert from and to uri encoded strings.
phated
force-pushed
the
support-uri-encoding
branch
from
1a32706 to
73379c5
Compare
phated
changed the title
|
@thlorenz I've rebased this and combined all the RegExp changes into a single modification of Please note that this also removed the |
|
Here is a visualization of the new regexp |
There was a problem hiding this comment.
Thanks for all the work putting this together.
Let's clean up the questions/requests I had and I'll have another look.
From what I can tell the regex looks fine, but just want to make sure the original behavior didn't change and we need the tests to be as unmodified as possible to be sure of that.
test/comment-regex.js
Outdated
| t.ok(comment(x, ''), 'matches ' + x) | ||
| t.ok(commentWithCharSet(x, ''), 'matches ' + x + ' with charset') | ||
| t.ok(commentWithCharSet(x, '', '='), 'matches ' + x + ' with charset') | ||
| t.ok(comment(x, '', convert.commentRegex), 'matches ' + x) |
There was a problem hiding this comment.
ditto here .. existing tests should be touched as little as possible to ensure nothing breaks
Co-authored-by: Thorsten Lorenz <[email protected]>
|
This introduces a subtle (but necessary) breaking change with the regexp. It will no longer parse a sourcemap that had the encoding specified at |
Fixes #59 without dropping the compatibility.