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

feat(commerce): make commerce sub-package generally available #4088

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Spuffynism
Copy link
Contributor

@Spuffynism Spuffynism commented Jun 12, 2024

Opening as draft because I'd like some advice on the message we include to indicate deprecation of the product-listing sub-package. I was thinking of something like:

@deprecated The product-listing sub-package is deprecated. Prefer using the commerce sub-package instead.

@anthonydelage @jpmarceau

Also, is there something else that needs to be done in headless apart from removing the comments?

CAPI-98

@developer-experience-bot
Copy link
Contributor

developer-experience-bot bot commented Jun 12, 2024

Pull Request Report

PR Title

✅ Title follows the conventional commit spec.

Live demo links

Bundle Size

File Old (kb) New (kb) Change (%)
case-assist 204.8 204.8 0
commerce 291.9 291.9 0
search 372.3 372.3 0
insight 352.4 352.4 0
product-listing 266.9 266.9 0
product-recommendation 170.8 170.8 0
recommendation 217.8 217.8 0
ssr 365.1 365.1 0

SSR Progress

Use case SSR (#) CSR (#) Progress (%)
search 39 44 89
recommendation 0 4 0
product-recommendation 0 10 0
product-listing 0 13 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
product-recommendation : missing SSR support
product-listing : missing SSR support
case-assist : missing SSR support
insight : missing SSR support
commerce : missing SSR support

@jpmarceau
Copy link
Contributor

jpmarceau commented Jun 12, 2024

How about:

@deprecated The product-listing sub-package is deprecated. Use the commerce sub-package instead.

As far as what's required for the new headless controllers, if we want to remove the beta comment, I think we'll just have to sync so that I remove analogous warnings on docs.coveo.com starting from the target version 👍

@Spuffynism
Copy link
Contributor Author

Spuffynism commented Jun 13, 2024

I've also deprecated the product recommendations sub-package as it's also replaced by the commerce one!

@Spuffynism Spuffynism marked this pull request as ready for review June 14, 2024 16:56
@Spuffynism Spuffynism requested review from a team as code owners June 14, 2024 16:56
@Spuffynism Spuffynism marked this pull request as draft June 14, 2024 16:56
@Spuffynism
Copy link
Contributor Author

Let's keep this as draft until we have remaining Commerce GA tasks complete.

@jpmarceau
Copy link
Contributor

I was curious and tried something I should have tried earlier: generating docs using the @deprecated tag. I noticed that it doesn't currently do anything, as far as documentation goes 🙃 It would be nice to add support for the tag, but for now I think we could just use plain text and the deprecation would be clear in the docs.
I still think that having the @internal tag can be useful for intellisense though. So, how about having both:

Deprecated. Use the commerce sub-package instead of the deprecated product-listing sub-package.
@deprecated

@Spuffynism
Copy link
Contributor Author

@jpmarceau did you mean to include @internal in your suggestion? I'm a bit confused? 🤔

@jpmarceau
Copy link
Contributor

@jpmarceau did you mean to include @internal in your suggestion? I'm a bit confused? 🤔

Yes, my thinking was to include @internal on its own line, for intellisense without interfering with docs. If you put the @internal tag on the same line as a doc string, that line isn't extracted for docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants