-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
We actually need to disable sourceMaps when we compile them as well #1796
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, why not update goja as well?
I wanted to also enable some tc39 tests with a new version and do it in another PR. But I already started running the tests so I can add to this one if you want to :) |
However you prefer to do it, but 👍 from me for a goja update today |
Codecov Report
@@ Coverage Diff @@
## master #1796 +/- ##
==========================================
- Coverage 71.57% 71.53% -0.05%
==========================================
Files 181 181
Lines 13939 13939
==========================================
- Hits 9977 9971 -6
- Misses 3331 3337 +6
Partials 631 631
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
btw I have no idea how source maps work precisely, but would this change be too difficult to test? |
It literally cares about a comment at the end of a source file //# sourceMappingURL=ajv.min.js.map |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but this is breaking change, no? Sourcemaps were supported before?
if they at any point worked in any capacity it wasn't intentional ;) |
No description provided.