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

@wordpress/api-fetch package has lodash as a dependency #39495

Closed
fabiankaegy opened this issue Mar 16, 2022 · 19 comments · Fixed by #41687
Closed

@wordpress/api-fetch package has lodash as a dependency #39495

fabiankaegy opened this issue Mar 16, 2022 · 19 comments · Fixed by #41687
Assignees
Labels
[Package] API fetch /packages/api-fetch [Type] Performance Related to performance efforts

Comments

@fabiankaegy
Copy link
Member

The @wordpress/api-fetch package is super useful even outside of the context of the editor. In theory it would be a great package to use on the frontend of any WordPress site to interact with the WordPress Rest API.

Currently however it relies on the @wordpress/url package which intern has a dependency on lodash. Because of that the size of the @wordpress/api-fetch npm package is 230 kB making it un suitable for the sage on the frontend.

@gziolo
Copy link
Member

gziolo commented Mar 16, 2022

There is a more high-level issue that opened a long time ago: #17025. I think there is a common agreement that we could replace lodash usage with native API wherever applicable. Should we consolidate the issue with the other one and close this one as duplicate?

@gziolo
Copy link
Member

gziolo commented Mar 16, 2022

There is also #16938 that is still open for @wordpress/element so it might need some more thinking on how we approach this growing inconvenience.

@fabiankaegy
Copy link
Member Author

For reference, this here is the only instance of lodash being used in the @wordpress/url package:

import { deburr, trim } from 'lodash';

@gziolo
Copy link
Member

gziolo commented Mar 16, 2022

String.prototype.trim() is a simple one to use as replacement.

deburr is a more tricky one.

@Mamaduka
Copy link
Member

We could probably have to re-implement remove_accents in JS and get rid of deburr.

Few folks also pointed out that deburr doesn't handle accent removal correctly.

@fabiankaegy
Copy link
Member Author

@Mamaduka I didn't know that PHP function existed. If we were to port it to JS where should it live? 🤔 Would it be it's own @wordpress/ package or should it live under compose / i18n ?

@gziolo
Copy link
Member

gziolo commented Mar 16, 2022

Do we use it outside of @wordpress/url? Otherwise, it can be internal for now.

@fabiankaegy
Copy link
Member Author

fabiankaegy commented Mar 16, 2022

deburr seems to be used in 9 files across the entire repo: https://github.com/WordPress/gutenberg/search?q=deburr

import { deburr } from 'lodash';

import { deburr, trim } from 'lodash';

import { deburr, trim } from 'lodash';

import { debounce, deburr, escapeRegExp } from 'lodash';

import { noop, deburr } from 'lodash';

import { deburr, differenceWith, find, words } from 'lodash';

import { escapeRegExp, find, deburr } from 'lodash';

But the usage in the url package is the only one I would consider one in a public package meant for consumption outside of the editor

@Mamaduka
Copy link
Member

We could potentially replace most of them with updated cleanForSlug.

@gziolo
Copy link
Member

gziolo commented Mar 16, 2022

We could potentially replace most of them with updated cleanForSlug.

The question would be if making @wordpress/url a dependency makes sense in all cases.

@Mamaduka
Copy link
Member

The question would be if making @wordpress/url a dependency makes sense in all cases.

Probably not, but we don't need to replace them all. We can start with cases where strings are used for slugs.

@gziolo
Copy link
Member

gziolo commented Mar 17, 2022

@wordpress/url alone is 3.78 kB without lodash according to https://bundlephobia.com/package/@wordpress/[email protected].

Probably not, but we don't need to replace them all. We can start with cases where strings are used for slugs.

I think that would be a good first step. We can decide later whether it makes sense to extract to its own package.

@tyxla
Copy link
Member

tyxla commented Jun 9, 2022

I'm planning to start working incrementally on removing Lodash completely from the repository. Usage is not so high, and the bundle size impact will be huge for some of the packages when used externally.

@fabiankaegy
Copy link
Member Author

@tyxla That is awesome to hear <3

The biggest hurdle for me when looking into removing lodash from the @wordpress/url package was that I couldn't find a great replacement for the deburr function.

If you find a way to remove that dependency that will have a huge impact on the ability to use various @wordpress/ packages on the frontend of a site.

@tyxla
Copy link
Member

tyxla commented Jun 9, 2022

Good to know @fabiankaegy 👍 We'll most likely end up having to introduce either a few new utility functions or reusing some well-established implementations that are broadly adopted by the community. deburr is definitely one of those, but throttle and debounce also immediately come to mind when talking about this. We'll get to them all eventually.

@fabiankaegy
Copy link
Member Author

I took a shot at #39495 (comment) but failed because I don't really have a good enough understanding of the underlying text encoding.

@tyxla
Copy link
Member

tyxla commented Jun 9, 2022

Interesting. Luckily, a few years ago I did implement an almost exact replica of the WP core remove_accents() for a side project: remove-accents.

It seems to be used quite extensively, and I couldn't be happier to spend more time maintaining it than if it was used in Gutenberg. It could be one way to get rid of deburr() usage, even if it meant releasing a new version with some improvements.

@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] API fetch /packages/api-fetch [Type] Performance Related to performance efforts
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants