Skip to content

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