use vendor libraries from npm and various fixes #3066
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
haarg
force-pushed
the
haarg/deps-from-npm
branch
from
2d176ad
to
46c602f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3066 +/- ##
==========================================
- Coverage 72.47% 71.71% -0.76%
==========================================
Files 69 70 +1
Lines 2372 2411 +39
Branches 334 338 +4
==========================================
+ Hits 1719 1729 +10
- Misses 526 555 +29
Partials 127 127
|
haarg
force-pushed
the
haarg/deps-from-npm
branch
from
46c602f
to
15dbf5e
Compare
haarg
force-pushed
the
haarg/deps-from-npm
branch
2 times, most recently
from
50e7211
to
882df87
Compare
|
@dhogan8 if you have a moment, can you take a quick look at the front end changes? The diff is too large for a human to handle, but even if you spent 5 or 10 minutes on it, that would be helpful. |
haarg
force-pushed
the
haarg/deps-from-npm
branch
from
882df87
to
9b25002
Compare
The new code doesn't use any inline code, and doesn't use jQuery.
This also updates the version from 2.0.5b to 2.31.3.
This requires modifying a vendor file, but it will soon be replaced by a newer version without this problem.
syntaxhighlighter is not entirely designed to be used from npm, so it requires some weird bits. For one, the brushes are separate packages with terrible names. The themes are also badly named, and not correctly packaged. The theme is written in scss and doesn't have a compiled version available via npm. This requires adding an scss compiler. In the future, it would be better to convert all of our code to scss, so this isn't terrible.
All of the uses of jQuery via window have been removed. The only remaining implicit use is the qtip2 module, which is covered by the injection mechanism.
haarg
force-pushed
the
haarg/deps-from-npm
branch
from
9b25002
to
4380596
Compare
oalders
approved these changes
May 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This removes the vendored libraries we had, and instead uses them from npm.
It also fixes various problems that were introduced with the conversion to esbuild. With that conversion, the jQuery object and other functions were no longer globally available. This broke various bits of inline code.
This removes all of the inline code, like
onclickhandlers.