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

Inaccurate JSdoc comment for page.size in astro/@types #11560

Closed
1 task done
ArmandPhilippot opened this issue Jul 26, 2024 · 4 comments · Fixed by #11561
Closed
1 task done

Inaccurate JSdoc comment for page.size in astro/@types #11560

ArmandPhilippot opened this issue Jul 26, 2024 · 4 comments · Fixed by #11561
Labels
- P2: nice to have Not breaking anything but nice to have (priority)

Comments

@ArmandPhilippot
Copy link
Contributor

Astro Info

Astro                    v4.12.2
Node                     v18.20.3
System                   Linux (x64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             none

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

I was reading the Routing guide and I noticed an error which leads me to astro/@types package.

The Page interface in astro/@types indicates that the default for page.size is 25. It seems to be wrong:

What's the expected result?

/**
 * Represents a single page of data in a paginated collection
 *
 * [Astro reference](https://docs.astro.build/en/reference/api-reference/#the-pagination-page-prop)
 */
export interface Page<T = any> {
	/** result */
	data: T[];
	/** metadata */
	/** the count of the first item on the page, starting from 0 */
	start: number;
	/** the count of the last item on the page, starting from 0 */
	end: number;
	/** total number of results */
	total: number;
	/** the current page number, starting from 1 */
	currentPage: number;
-	/** number of items per page (default: 25) */
+	/** number of items per page (default: 10) */
	size: number;
	/** number of last page */
	lastPage: number;
	url: {
		/** url of the current page */
		current: string;
		/** url of the previous page (if there is one) */
		prev: string | undefined;
		/** url of the next page (if there is one) */
		next: string | undefined;
		/** url of the first page (if the current page is not the first page) */
		first: string | undefined;
		/** url of the next page (if the current page in not the last page) */
		last: string | undefined;
	};
}

Link to Minimal Reproducible Example

https://stackblitz.com/edit/astro-paginate-pagesize?file=src%2Fpages%2Fpokemon%2F[page].astro&on=stackblitz

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Jul 26, 2024
ArmandPhilippot added a commit to ArmandPhilippot/astro that referenced this issue Jul 26, 2024
10 is the correct default pageSize and not 25.

Closes withastro#11560
ArmandPhilippot added a commit to ArmandPhilippot/astro that referenced this issue Jul 26, 2024
10 is the correct default pageSize and not 25.

Closes withastro#11560
@bluwy
Copy link
Member

bluwy commented Jul 26, 2024

You're right. Feel free to send a PR to fix that 👍

@bluwy bluwy added - P2: nice to have Not breaking anything but nice to have (priority) and removed needs triage Issue needs to be triaged labels Jul 26, 2024
@ArmandPhilippot
Copy link
Contributor Author

@bluwy Done! I just created the issue to keep a track (I don't know if this was needed...).

@bluwy
Copy link
Member

bluwy commented Jul 26, 2024

For single line changes it may be easier (especially for you as there's less work) to create a PR directly in the future, as long as the PR explains the context too! But still it doesn't hurt if you prefer to open an issue first.

@ArmandPhilippot
Copy link
Contributor Author

Ok @bluwy, thanks for you feedback! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: nice to have Not breaking anything but nice to have (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants