feast-dev / feast

The Open Source Feature Store for Machine Learning
https://feast.dev
Apache License 2.0
5.59k stars 998 forks source link

Get rid of duplicate peer dependencies at Feast UI #3785

Closed usersavchenko closed 1 month ago

usersavchenko commented 1 year ago

Currently there is problem, Feast UI Obliges us to download a pile of various peer dependencies. They are duplicates of "dependencies" as you can see from my screenshot. What the use of this approach ? Can we release users from downloading all of these ones ?

image

lokeshrangineni commented 6 months ago

@usersavchenko - i don't consider myself a UI developer so please take my comment with grain of salt.

Feast UI can be used as a standalone project or a plugin in an existing react project. AFAIK peer dependencies are downloaded if the feast is used as plugin.

If you are using feast as a standalone project then only dependencies list will be downloaded.

The expectation is only either peerDepedencies or dependencies used depending on how you are using feast UI components.

Could you check what do you get with the command npm list? see if the dependencies are downloaded twice.

My comment is based on the stackoverflow discussion.

peruukki commented 1 month ago

I was also wondering about the large number of peer dependencies, thanks for the clarification.

Maybe the peerDependencies list could still be trimmed down a bit. They are meant for framework-like dependencies that should be managed by the "host" app, and usually there needs to be a single instance of them. dependencies are always installed regardless of how the package is used.

At least the @types, react-scripts and typescript packages are only required for the Feast UI development, so they don't necessarily need to be installed by the application. From the runtime dependencies, possibly at least moment, prop-types, query-string, use-query-params or zod could be omitted.

shuchu commented 1 month ago

Hi All, the CRA is deprecated. Any suggestion about UI code should move forward? There is a package named nth-check is a dependent of react-scripts, and it has a high severity security issue.

On Sat, Sep 14, 2024 at 08:23 Harri Lehtola @.***> wrote:

I was also wondering about the large number of peer dependencies, thanks for the clarification.

Maybe the peerDependencies list could still be trimmed down a bit. They are meant for framework-like dependencies that should be managed by the "host" app, and usually there needs to be a single instance of them. dependencies are always installed regardless of how the package is used.

At least the @types, react-scripts and typescript packages are only required for the Feast UI development, so they don't necessarily need to be installed by the application. From the runtime dependencies, possibly at least moment, prop-types, query-string, use-query-params or zod could be omitted.

— Reply to this email directly, view it on GitHub https://github.com/feast-dev/feast/issues/3785#issuecomment-2350990358, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALUUM7RJYEVKIYQNSSYO6DZWQ2E5AVCNFSM6AAAAAA5ZBU7J6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJQHE4TAMZVHA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

peruukki commented 1 month ago

I suppose the first step would be to eject and make the setup work with that. Then it would be possible to remove unnecessary depedencies and possibly switch any outdated ones..

peruukki commented 1 month ago

I've been trying to narrow down the peer dependencies, it's a bit tricky because how many of them you need depends on how much you want to customize. But I'd categorize the current peer dependencies like this:

Clear peer dependencies

react, react-dom: These are needed for any React web app, and having a single React version across your app is highly recommended.

Possible peer dependencies

@elastic/eui, @emotion/react: If you are adding custom tabs, it makes sense to use the same component library and CSS-in-JS solution as Feast UI, and preferably the same versions and/or single instances (though I don't know how important that is). Though using only @elastic/eui of these may well be enough. And you don't need these if you only want a custom projectListPromise. So I'd mark these as optional peer dependencies.

Not peer dependencies

@elastic/datemath, d3, inter-ui, moment, prop-types (actually I don't think is used by Feast UI anymore), query-string, react-query, react-router-dom, react-scripts, use-query-params, zod: These packages are for more specific purposes, and many of them you wouldn't necessarily need in your app with the current customization possibilities, and you could use different versions or even other alternatives.

Does this make sense? I could create a PR.

tokoko commented 1 month ago

+1 for aggressively trimming down the list. I doubt anyone is actually using the UI as a plugin today, we can worry about the peer dependencies when we actually run into some problems. Feel free to open a PR, thanks.