Skip to content
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

Remove lodash #2158

Merged
merged 2 commits into from
Dec 2, 2024
Merged

Remove lodash #2158

merged 2 commits into from
Dec 2, 2024

Conversation

marcustyphoon
Copy link

@marcustyphoon marcustyphoon commented Nov 16, 2024

We have a vendored lodash dependency, but it's only used in three unimportant places and contains an eval call. I think it would make sense to remove it.

The only things it's used for are:

  • fix_autoplaying_yanked_videos/fix_jk_scrolling in XKit Patches, injected functions that are already conditional on lodash's existence (and that, I assume, do nothing now). Not changed.
  • XKit.tools.normalize_indentation, which is only used when creating script elements (where pretty printing doesn't matter) and which I cannot be bothered to spend time trying to comprehend the implementation of. Function and references removed in xkit.js; not changed in the XKit Patches entry for v7.9.2 because as of Move 7.9.2 patches into 7.10.0 xkit.js #2165/v7.9.2: Fix XKit Patches on any future versions #2166 the patches will not be run on 7.10.0 and future versions which lack lodash.
  • Allowing XKit.css_map to be destructured (const {keyToCss, descendantSelector} = XKit.css_map), which is sugar and is only used in Fresh Prince. Function call removed in xkit.js and not changed in the XKit Patches entry for v7.9.2 for the above reason; destructuring removed in Fresh Prince, which should technically be deployed to v7.9.2 in advance for seamless upgrade experience, but it's Fresh Prince, so it's really not worth bothering (the hypothetical daily Fresh Prince user will be automatically updated by XKit Update shortly after installing v7.10.0 and they can manually update earlier if they really want).

  • test the upgrade path to this (xkit patches must not crash)

@marcustyphoon marcustyphoon marked this pull request as draft November 22, 2024 19:50
@marcustyphoon marcustyphoon force-pushed the no-lodash branch 2 times, most recently from 335504a to dd46044 Compare November 22, 2024 20:18
@marcustyphoon marcustyphoon marked this pull request as ready for review November 22, 2024 20:19
@marcustyphoon

This comment was marked as outdated.

@marcustyphoon marcustyphoon marked this pull request as draft November 22, 2024 20:24
@marcustyphoon marcustyphoon marked this pull request as ready for review December 2, 2024 11:22
@marcustyphoon marcustyphoon requested a review from hobinjk December 2, 2024 11:25
@marcustyphoon marcustyphoon merged commit c88838b into new-xkit:master Dec 2, 2024
1 check passed
@marcustyphoon marcustyphoon deleted the no-lodash branch December 2, 2024 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants