search

coveo / ui-kit

Coveo UI kit repository, home of @coveo/headless, @coveo/atomic, and more.
Apache License 2.0
51 stars 34 forks source link

chore(atomic, headless): make bueno external #4433

Closed fpbrault closed 1 week ago

fpbrault commented 1 week ago

This PR makes bueno external in atomic and headless.

https://coveord.atlassian.net/browse/KIT-3551

github-actions[bot] commented 1 week ago

Pull Request Report

PR Title

:white_check_mark: Title follows the conventional commit spec.

Live demo links

Bundle Size

File Old (kb) New (kb) Change (%)
case-assist 239.2 235.9 -1.4
commerce 338.1 339.1 0.3
search 407.7 409.5 0.4
insight 394.2 395.4 0.3
recommendation 250.1 248.5 -0.6
ssr 401.7 403.5 0.5
ssr-commerce 350.3 351.4 0.3

SSR Progress

Use case SSR (#) CSR (#) Progress (%)
search 39 44 89
recommendation 0 4 0
case-assist 0 6 0
insight 0 27 0
commerce 0 15 0
Detailed logs search : buildInteractiveResult
search : buildInteractiveInstantResult
search : buildInteractiveRecentResult
search : buildInteractiveCitation
search : buildGeneratedAnswer
recommendation : missing SSR support
case-assist : missing SSR support
insight : missing SSR support
commerce : missing SSR support
fpbrault commented 1 week ago

why is this needed again ? I don't see the point of externalizing bueno. If I were to make a library that uses zod for schema validation, I feel like I would bundle those validation functions with my library. This is not a framework headless and atomic depend on to work. This is a library for utility functions.

The main reason to externalize Bueno was to avoid code duplication when a end-user bundles Atomic or Headless in their own project: https://coveord.atlassian.net/browse/KIT-3181 I dont know if there was other reasons @louis-bompart ?

alexprudhomme commented 1 week ago

why is this needed again ? I don't see the point of externalizing bueno. If I were to make a library that uses zod for schema validation, I feel like I would bundle those validation functions with my library. This is not a framework headless and atomic depend on to work. This is a library for utility functions.

The main reason to externalize Bueno was to avoid code duplication when a end-user bundles Atomic or Headless in their own project: https://coveord.atlassian.net/browse/KIT-3181 I dont know if there was other reasons @louis-bompart ?

But do users really install bueno ? If they don't, they should not have duplication right? I feel like bueno is more an internal package for us that we made before there were popular schema validation libraries like zod.

fpbrault commented 1 week ago

why is this needed again ? I don't see the point of externalizing bueno. If I were to make a library that uses zod for schema validation, I feel like I would bundle those validation functions with my library. This is not a framework headless and atomic depend on to work. This is a library for utility functions.

The main reason to externalize Bueno was to avoid code duplication when a end-user bundles Atomic or Headless in their own project: https://coveord.atlassian.net/browse/KIT-3181 I dont know if there was other reasons @louis-bompart ?

But do users really install bueno ? If they don't, they should not have duplication right? I feel like bueno is more an internal package for us that we made before there were popular schema validation libraries like zod.

Thats a good point, I see ~20k weekly downloads on npmjs.org, but I would not be surprised if these are all from us/our CI...

louis-bompart commented 1 week ago

why is this needed again ? I don't see the point of externalizing bueno. If I were to make a library that uses zod for schema validation, I feel like I would bundle those validation functions with my library. This is not a framework headless and atomic depend on to work. This is a library for utility functions.

The main reason to externalize Bueno was to avoid code duplication when a end-user bundles Atomic or Headless in their own project: https://coveord.atlassian.net/browse/KIT-3181 I dont know if there was other reasons @louis-bompart ?

But do users really install bueno ? If they don't, they should not have duplication right? I feel like bueno is more an internal package for us that we made before there were popular schema validation libraries like zod.

Thats a good point, I see ~20k weekly downloads on npmjs.org, but I would not be surprised if these are all from us/our CI...

It is very unlikely that we have that many users for Bueno (, one of these days, we might want to consider putting the 🪓 into Bueno in favour of another mainstream library like Zod.

However, the concept/philosophy here still applies:

louis-bompart commented 1 week ago

I took the liberty of fixing those poor tests (& removing a few cats from the code 😿). I peppered my changes with comments to explain the gist of it, can talk you thru tmrw if you want! https://github.com/coveo/ui-kit/pull/4433/commits/d2aa4e5eda30e76d6e385f60f43fb6f2e0d48ce7