-
-
Notifications
You must be signed in to change notification settings - Fork 412
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
Use modern jsonlint fork #1018
Use modern jsonlint fork #1018
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1018 +/- ##
==========================================
+ Coverage 59.84% 63.48% +3.63%
==========================================
Files 104 104
Lines 3011 5773 +2762
Branches 680 1727 +1047
==========================================
+ Hits 1802 3665 +1863
- Misses 1209 2107 +898
- Partials 0 1 +1 ☔ View full report in Codecov by Sentry. |
This looks good assuming it is working as expected, were there other mentioning of this package that can be removed? Maybe in the rollup configuration? |
@HarelM I got the error reporting working. It's now using the error from the catch(err) now instead of the parseError hack For instance setting below, where there is inserted an {
"id": "background",
"type": "background",
"paint": {
"background-color": m"#D8F2FF"
},
"filter": ["all"],
"layout": {"visibility": "visible"},
"maxzoom": 24
} Before After |
c77e5ee
to
80d34c3
Compare
I abstracted this error handling out. It's the exact same function we had twice for some reason |
Is it possible to add a test to cover this scenario? |
@HarelM , I managed to add a test that break the json syntax, and checks that the red error circle appear in the side of the editor It passes on both main and this PR. |
Thanks! |
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.
Just remove the "only" and you can merge it, thanks!!
We have a few vulnerabilities mentioned when installing, due to obsolete versions of Underscore, which comes from a 10y old version of jsonlint.
This PR swap the old jsonlint from 2015:
https://github.com/josdejong/jsonlint
With this one:
https://github.com/prantlf/jsonlint
Launch Checklist
CHANGELOG.md
under the## main
section.