Skip to content

remove lodash.escaperegexp, lodash.groupby, and lodash.partition depe… - #1255

Open
SebbeBroman wants to merge 2 commits into
C2FO:mainfrom
SebbeBroman:main
Open

remove lodash.escaperegexp, lodash.groupby, and lodash.partition depe…#1255
SebbeBroman wants to merge 2 commits into
C2FO:mainfrom
SebbeBroman:main

Conversation

@SebbeBroman

Copy link
Copy Markdown

Removing the last lodash dependencies, should reduce bundle size especially for the parse package.

This would make format and parse dependency free.

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@c2fo-cibot c2fo-cibot Bot added the size/M Denotes a PR that changes 30-99 lines label Aug 4, 2026

@gameroman gameroman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice 👍

Comment on lines +4 to +7
/** Escape special characters for use in a RegExp. */
const escapeRegExp = (value: string): string => {
return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also once Node 22 is EOL we can use native RegExp.escape

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I did not want to change the supported node version, but this should be functionally the same as lodash. But RegExp.excape is a bit more extensive so that would be a good replacement.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, just saying that we'll able to change it in the future

const grouped = filteredHeaders.reduce<Record<string, string[]>>((acc, header) => {
(acc[header] ||= []).push(header);
return acc;
}, {});

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be Object.groupBy in the future, but that would change the minimum node version.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a // TODO(major): ... comment to not forget

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comments too now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Denotes a PR that changes 30-99 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants