-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Replace lodash 'sortBy' usage with Array.prototype.sort #11810
Closed
jayaddison
wants to merge
33
commits into
babel:main
from
jayaddison:dependencies/reduce-lodash-usage-sortby
Closed
Changes from 18 commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
e003efe
Replace lodash 'sortBy' usage with Array.prototype.sort
jayaddison 13fce2a
Rectify comment
jayaddison 5a38c24
Extract priority as local function
jayaddison 67a41ad
Update packages/babel-core/src/transformation/block-hoist-plugin.js
jayaddison 47cffbe
Flip Array.prototype.sort comparator argument order
jayaddison 65099df
Switch from Array.prototype.every to Array.prototype.find
jayaddison 5ce6cf8
Ensure stable element sort order for Node < 12 by retaining a map of …
jayaddison 3031dbf
Fixup: null rather than false argument to Map constructor when sortin…
jayaddison e84d4bb
Update packages/babel-core/src/transformation/block-hoist-plugin.js
jayaddison a7e7a71
Nit: rephrase boolean fallback behaviour to ensure that sortIsUnstabl…
jayaddison 8617a7f
Nit: remove empty newline
jayaddison 8a2e5a8
Refactor stabilityMap construction and comparator logic
jayaddison a472de9
Ensure that the sort comparator always returns a value
jayaddison fdd2756
Pre-check whether NodeJS 'process' variable is defined
jayaddison 9e2db88
Fixup: linting
jayaddison 3ee06b3
Update logical condition: use map as a tie-breaker for non-NodeJS env…
jayaddison e50df93
Simplification: use Array.prototype.some instead of Array.prototype.f…
jayaddison 8cc9106
Simplify expression: invoke Object.entries instead of using an equiva…
jayaddison 55b6994
Revert "Simplify expression: invoke Object.entries instead of using a…
jayaddison d213549
Merge branch 'main' into dependencies/reduce-lodash-usage-sortby
jayaddison 264d0f3
Add key, value types to Map constructor
jayaddison f681c41
Use semver to perform version parsing and comparison
jayaddison fad44b7
Fixup: misplaced trailing semi-colon
jayaddison 3fdb680
Fixup: add missing semver import
jayaddison f5764fa
Read node version from 'process.versions.node'
jayaddison 3ddc74a
Use static string version instead of attempting semver parsing (from …
jayaddison af82e70
babel-core: update to semver@^7.3.2
jayaddison 1d7e4f3
Skip flow type-checking for stabilityMap arithmetic
jayaddison 2465432
Add flow type declaration for semver.clean
jayaddison bef7e93
Revert "babel-core: update to semver@^7.3.2"
jayaddison d468331
Add value presence check for nodeVersion
jayaddison 26a35d1
lint: apply fixes suggested by prettier
jayaddison 4f5673b
lint: apply string quoting fix suggested by prettier
jayaddison File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
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
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.
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.
Feel free to ignore this suggestion, just wanted to point out that the conditional can be simplified if you make browsers produce
nodeVersion === 0
.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.
Thanks @coreyfarrell - I think I'd prefer to keep the meaning/semantics of
nodeVersion
closer to the ground truth (i.e. it'll genuinely have an undefined value in non-Node environments), even though I'd admit that it's (hopefully) unlikely that anyone would be line-by-line debugging the value. I'm sure I've probably used tricks like this to simplify my own code elsewhere too, so it's a bit arbitrary of me not to accept. But I think I'd prefer the more verbose condition in this case.