apache / jena

Apache Jena
https://jena.apache.org/
Apache License 2.0
1.12k stars 652 forks source link

Gh 2767 Fix Fuseki UI JavaScript warnings #2768

Closed kinow closed 1 month ago

kinow commented 1 month ago

GitHub issue resolved #2767

Pull request Description:

This last point is the most important one. We had some code modifying codemirror, which comes via Zazuko libs, but we never declared in package.json, using only as transitive. Now it's listed in our code as we actually import that code and modify it for our Query component.

But the most important change was replacing query-string by qs. We had both, in different parts of the code, but luckily we had unit tests covering that, so replacing was really easy and doesn't seem to break anything.

Choosing between the two was not straightforward as none is maintained by a big community or known company. After some searching, and comparing usage & downloads, looks like qs has a larger user base, and since its description mentions "security", I thought it'd be OK to keep that one and drop query-string (which might still be used in Zazuko components... although looks like they also have qs somewhere in their other transitive dependencies -- JS dependencies are complicated :sighs:)

I grouped each change into separate commits so that I can revert/drop any if needed. Let me know if these should be squashed before the merge, and I will do so :+1:


By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.


See the Apache Jena "Contributing" guide.

kinow commented 1 month ago

Unit & e2e & lint & build passing locally for me (e2e and unit both failed after I first pushed, so really thankfully our JS code is reasonably well covered as I had no recollection of why the test would fail :pray: )