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

[Feature]: More description for target version #1000

Closed
manudeli opened this issue Jun 28, 2024 · 9 comments · Fixed by #1024
Closed

[Feature]: More description for target version #1000

manudeli opened this issue Jun 28, 2024 · 9 comments · Fixed by #1024
Assignees
Labels
bug Something isn't working

Comments

@manudeli
Copy link
Member

Package Scope

@suspensive/react-query

Bug description

image

issued by @juno7803

Expected behavior

No response

To Reproduce

No response

Possible Solution

No response

etc.

No response

@manudeli manudeli added the bug Something isn't working label Jun 28, 2024
@manudeli
Copy link
Member Author

manudeli commented Jun 28, 2024

@juno7803 Could you add what you expect as Pull Request?

@juno7803
Copy link
Member

juno7803 commented Jun 28, 2024

I believe that the cli for @suspensive/react-query status clearly supports different versions of react-query for users.

However, despite its clear functionality, I find the guidance provided by the CLI to be unclear. If it's alright with you, could I contribute by submitting a Pull Request? 😁

@gwansikk
Copy link
Collaborator

I believe that the cli for @suspensive/react-query status clearly supports different versions of react-query for users.

However, despite its clear functionality, I find the guidance provided by the CLI to be unclear. If it's alright with you, could I contribute by submitting a Pull Request? 😁

@juno7803 I agree, especially the (5) in [@suspensive/react-query] v2.3.0 (5) is not clear. It is difficult for users to understand what it means. Please revise it to provide sufficient guidance from the user's perspective.

@manudeli
Copy link
Member Author

manudeli commented Jun 28, 2024

Suggestion

I made example what I expected.
When I use suspensive-react-query status, I felt that I want to know what interface will be exported by each packages(@suspensive/react-query, @tanstack/react-query)

Suspensive React Query with TanStack Query v4

Suspensive React Query status:
@suspensive/react-query@2.3.0 export @suspensive/react-query-4's interfaces
  - useSuspenseQuery
  - useSuspenseQueries
  - useSuspenseInfiniteQuery
  - queryOptions
  - SuspenseQuery
  - SuspenseQueries
  - SuspenseInfiniteQuery
  - QueryErrorBoundary
@tanstack/react-query@4.36.2
  - all interfaces

Suspensive React Query with TanStack Query v5

Suspensive React Query status:
@suspensive/react-query@2.3.0 export @suspensive/react-query-5's interfaces
  - SuspenseQuery
  - SuspenseQueries
  - SuspenseInfiniteQuery
  - QueryErrorBoundary
@tanstack/react-query@5.32.0
  - useSuspenseQuery
  - useSuspenseQueries
  - useSuspenseInfiniteQuery
  - queryOptions
  - all interfaces

If you have better Idea, Suggest example before developing please! @juno7803 @gwansikk

@gwansikk
Copy link
Collaborator

gwansikk commented Jun 28, 2024

@juno7803 @manudeli

#1000 (comment)

Oh, this is really detailed and impressive.

  • However, displaying the list of interfaces might reduce readability if more interfaces are added in the future.
  • Additionally, we would need to update the CLI list every time a new interface is added, which adds to the maintenance workload.

How about including a link to the official documentation for information about the interfaces?

@manudeli
Copy link
Member Author

How about including a link to the official documentation for information about the interfaces?

In my opinion, Linking official doc is not good idea to expose what exactly dist/index.js export at each version because official doc only expose latest public apis

So In my opinion, to implement detailed suspensive-react-query status Parsing dist/index.js of @suspensive/react-query's exposing package(@suspensive/react-query-4, @suspensive/react-query-5) is good way to expose public apis of each version

However, displaying the list of interfaces might reduce readability if more interfaces are added in the future.

I agree with your opinion, then how about showing public apis of @suspensive/react-query only, not @tanstack/react-query

Suspensive React Query with TanStack Query v4

Suspensive React Query status:
@suspensive/react-query@2.3.0 export @suspensive/react-query-4's interfaces
  - useSuspenseQuery
  - useSuspenseQueries
  - useSuspenseInfiniteQuery
  - queryOptions
  - SuspenseQuery
  - SuspenseQueries
  - SuspenseInfiniteQuery
  - QueryErrorBoundary
@tanstack/react-query@4.36.2

Suspensive React Query with TanStack Query v5

Suspensive React Query status:
@suspensive/react-query@2.3.0 export @suspensive/react-query-5's interfaces
  - SuspenseQuery
  - SuspenseQueries
  - SuspenseInfiniteQuery
  - QueryErrorBoundary
@tanstack/react-query@5.32.0

@gwansikk
Copy link
Collaborator

So In my opinion, to implement detailed suspensive-react-query status Parsing dist/index.js of @suspensive/react-query's exposing package(@suspensive/react-query-4, @suspensive/react-query-5) is good way to expose public apis of each version

@manudeli I agree with your opinion! Is the method you explained the same as the one written below? Do we provide API information by branching according to the version, or do we dynamically display the code exported by dist/index.cjs using TypeScript AST?

if (suspensiveReactQueryVersion === '5') {
  console.log(`
- SuspenseQuery
- SuspenseQueries
- SuspenseInfiniteQuery
- QueryErrorBoundary`)
} else if (suspensiveReactQueryVersion === '4') {
  console.log(`
- useSuspenseQuery
- useSuspenseQueries
- useSuspenseInfiniteQuery
- queryOptions
- SuspenseQuery
- SuspenseQueries
- SuspenseInfiniteQuery
- QueryErrorBoundary`)
}

@manudeli
Copy link
Member Author

manudeli commented Jun 30, 2024

@gwansikk

Do we provide API information by branching according to the version, or do we dynamically display the code exported by dist/index.cjs using TypeScript AST?

In my opinion, display dynamically finally is better for me. but if we need to resolve this issue first, I think that displaying public apis by branching according to the version statically will be good solution at this first time!

Fast resolve issue, and improving feature gradually also good!

@juno7803 How do you think of this way?

@juno7803
Copy link
Member

juno7803 commented Jul 1, 2024

You guys shared a lot of valuable opinions! Thank you so much, and I apologize for the delay.

The biggest inconvenience from the user's perspective was understanding what CLI were trying to convey. Therefore, assuming you know the following key features of @suspensive/react-query, it was suggested to show what CLI status supports:

  • To use @tanstack/react-query and suspense more effectively
  • They can be used together, and even in lower versions(@tanstack/react-query), the latest APIs are provided.

At first, instead of specifying which APIs are provided when using with @tanstack/react-query v4 and which are provided when using with @tanstack/react-query v5, I thought to be sufficient to simply state that v4 provides v5 features and that when upgrading to v5, redundant APIs are maintained as singletons, @suspensive/react-query only exporting what is necessary.

However, after reading the new issue comments, I think it's best to be clear as @manudeli mentioned. (Upon reflection, I realize I have some background knowledge from @manudeli 's explanations of the library concept). While I completely agree with @gwansikk's opinion on readability and maintainability, providing document links is also part of maintainability, but personally, I don't prefer navigating through multiple layers (I think that might feel like depth to some people.), especially in CLI.

Therefore, as @manudeli suggested, I think it's best to first resolve the issue by displaying it statically and then improve it through automation. As for how to automate, I might need to study more to contribute as much as those who developed it..!

Thank you once again for sharing your valuable opinions! ❤️

@gwansikk gwansikk linked a pull request Jul 1, 2024 that will close this issue
1 task
manudeli pushed a commit that referenced this issue Jul 1, 2024
# Overview

I've added you as a co-author! @juno7803 
Thank you for your valuable feedback! Thanks to you, the explanation has
become even better.

I have made the API display static for now. In the next version, I will
make it dynamic to list and display the APIs.
#1000 (comment)

| version 4 | version 5|
| --- | --- |
|<img width="472" alt="image"
src="https://github.com/toss/suspensive/assets/39869096/2e4e4adf-c2db-4b19-bab4-e847075aae09">|<img
width="475" alt="image"
src="https://github.com/toss/suspensive/assets/39869096/1a5f6afa-d2a5-498b-99f6-a10049ea3fd1">|

## PR Checklist

- [x] I did below actions if need

1. I read the [Contributing
Guide](https://github.com/toss/suspensive/blob/main/CONTRIBUTING.md)
2. I added documents and tests.

---------

Co-authored-by: Jun <26808056+juno7803@users.noreply.github.com>
manudeli pushed a commit that referenced this issue Aug 3, 2024
# Overview

I've added you as a co-author! @juno7803 
Thank you for your valuable feedback! Thanks to you, the explanation has
become even better.

I have made the API display static for now. In the next version, I will
make it dynamic to list and display the APIs.
#1000 (comment)

| version 4 | version 5|
| --- | --- |
|<img width="472" alt="image"
src="https://github.com/toss/suspensive/assets/39869096/2e4e4adf-c2db-4b19-bab4-e847075aae09">|<img
width="475" alt="image"
src="https://github.com/toss/suspensive/assets/39869096/1a5f6afa-d2a5-498b-99f6-a10049ea3fd1">|

## PR Checklist

- [x] I did below actions if need

1. I read the [Contributing
Guide](https://github.com/toss/suspensive/blob/main/CONTRIBUTING.md)
2. I added documents and tests.

---------

Co-authored-by: Jun <26808056+juno7803@users.noreply.github.com>
manudeli pushed a commit that referenced this issue Aug 3, 2024
# Overview

I've added you as a co-author! @juno7803 
Thank you for your valuable feedback! Thanks to you, the explanation has
become even better.

I have made the API display static for now. In the next version, I will
make it dynamic to list and display the APIs.
#1000 (comment)

| version 4 | version 5|
| --- | --- |
|<img width="472" alt="image"
src="https://github.com/toss/suspensive/assets/39869096/2e4e4adf-c2db-4b19-bab4-e847075aae09">|<img
width="475" alt="image"
src="https://github.com/toss/suspensive/assets/39869096/1a5f6afa-d2a5-498b-99f6-a10049ea3fd1">|

## PR Checklist

- [x] I did below actions if need

1. I read the [Contributing
Guide](https://github.com/toss/suspensive/blob/main/CONTRIBUTING.md)
2. I added documents and tests.

---------
manudeli pushed a commit that referenced this issue Aug 3, 2024
# Overview

I've added you as a co-author! @juno7803 
Thank you for your valuable feedback! Thanks to you, the explanation has
become even better.

I have made the API display static for now. In the next version, I will
make it dynamic to list and display the APIs.
#1000 (comment)

| version 4 | version 5|
| --- | --- |
|<img width="472" alt="image"
src="https://github.com/toss/suspensive/assets/39869096/2e4e4adf-c2db-4b19-bab4-e847075aae09">|<img
width="475" alt="image"
src="https://github.com/toss/suspensive/assets/39869096/1a5f6afa-d2a5-498b-99f6-a10049ea3fd1">|

## PR Checklist

- [x] I did below actions if need

1. I read the [Contributing
Guide](https://github.com/toss/suspensive/blob/main/CONTRIBUTING.md)
2. I added documents and tests.

---------

Co-authored-by: Jun <26808056+juno7803@users.noreply.github.com>
manudeli pushed a commit that referenced this issue Aug 3, 2024
# Overview

I've added you as a co-author! @juno7803 
Thank you for your valuable feedback! Thanks to you, the explanation has
become even better.

I have made the API display static for now. In the next version, I will
make it dynamic to list and display the APIs.
#1000 (comment)

| version 4 | version 5|
| --- | --- |
|<img width="472" alt="image"
src="https://github.com/toss/suspensive/assets/39869096/2e4e4adf-c2db-4b19-bab4-e847075aae09">|<img
width="475" alt="image"
src="https://github.com/toss/suspensive/assets/39869096/1a5f6afa-d2a5-498b-99f6-a10049ea3fd1">|

## PR Checklist

- [x] I did below actions if need

1. I read the [Contributing
Guide](https://github.com/toss/suspensive/blob/main/CONTRIBUTING.md)
2. I added documents and tests.

---------

Co-authored-by: Jun <26808056+juno7803@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants