From c42fde65c3fcdc6139e95dc4a96c31758400d182 Mon Sep 17 00:00:00 2001
From: Philipp <ppetzold@users.noreply.github.com>
Date: Fri, 17 Mar 2023 21:39:13 +0100
Subject: [PATCH] drop take and skip (#545)

BREAKING CHANGE: We remove `paginationType` from config and always use limit/offset. Most current issues seem to be related take/skip. The alleged side effects of limit/offset on many-to-many relations seem to not exist anymore as coverage shows.
---
 README.md            | 14 --------------
 src/paginate.spec.ts | 12 ++++++++----
 src/paginate.ts      | 20 +-------------------
 3 files changed, 9 insertions(+), 37 deletions(-)

diff --git a/README.md b/README.md
index 6be7fc3..100ed2c 100644
--- a/README.md
+++ b/README.md
@@ -254,20 +254,6 @@ const paginateConfig: PaginateConfig<CatEntity> {
    */
   origin: 'http://cats.example',
 
-  /**
-   * Required: false
-   * Type: string
-   * Description: Allow user to choose between limit/offset and take/skip.
-   * Default: PaginationType.TAKE_AND_SKIP
-   *
-   * However, using limit/offset can return unexpected results.
-   * For more information see:
-   * [#477](https://github.com/ppetzold/nestjs-paginate/issues/477)
-   * [#4742](https://github.com/typeorm/typeorm/issues/4742)
-   * [#5670](https://github.com/typeorm/typeorm/issues/5670)
-   */
-  paginationType: PaginationType.LIMIT_AND_OFFSET,
-
   /**
    * Required: false
    * Type: boolean
diff --git a/src/paginate.spec.ts b/src/paginate.spec.ts
index c9dd915..a273957 100644
--- a/src/paginate.spec.ts
+++ b/src/paginate.spec.ts
@@ -548,9 +548,9 @@ describe('paginate', () => {
         const result = await paginate<CatEntity>(query, catRepo, config)
 
         expect(result.meta.search).toStrictEqual('Mouse')
-        const toy = clone(catToys[2])
+        const toy = clone(catToys[1])
         delete toy.cat
-        const toy2 = clone(catToys[1])
+        const toy2 = clone(catToys[2])
         delete toy2.cat
 
         expect(result.data).toStrictEqual([Object.assign(clone(cats[0]), { toys: [toy2, toy] })])
@@ -942,7 +942,7 @@ describe('paginate', () => {
             delete copy.cat
             return copy
         })
-        copyCats[0].toys = [copyToys[0], copyToys[1], copyToys[2]]
+        copyCats[0].toys = [copyToys[0], copyToys[2], copyToys[1]]
         copyCats[1].toys = [copyToys[3]]
 
         const orderedCats = [copyCats[3], copyCats[1], copyCats[2], copyCats[0], copyCats[4]]
@@ -1967,6 +1967,7 @@ describe('paginate', () => {
         await catRepo.softDelete({ id: cats[0].id })
         const result = await paginate<CatEntity>(query, catRepo, config)
         expect(result.meta.totalItems).toBe(cats.length)
+        await catRepo.restore({ id: cats[0].id })
     })
 
     it('should return only undeleted items', async () => {
@@ -1980,6 +1981,7 @@ describe('paginate', () => {
         await catRepo.softDelete({ id: cats[0].id })
         const result = await paginate<CatEntity>(query, catRepo, config)
         expect(result.meta.totalItems).toBe(cats.length - 1)
+        await catRepo.restore({ id: cats[0].id })
     })
 
     it('should return the specified columns only', async () => {
@@ -2064,7 +2066,9 @@ describe('paginate', () => {
 
         const result = await paginate<CatEntity>(query, catRepo, config)
 
-        expect(result.data.length).toBe(4)
+        expect(result.meta.totalItems).toBe(5)
+        expect(result.data.length).toBe(5)
+        expect(result.data[0].friends.length).toBe(4)
     })
 
     it('should return eager relations when set the property `loadEagerRelations` as true', async () => {
diff --git a/src/paginate.ts b/src/paginate.ts
index a85a045..42a7eb5 100644
--- a/src/paginate.ts
+++ b/src/paginate.ts
@@ -54,11 +54,6 @@ export class Paginated<T> {
     }
 }
 
-export enum PaginationType {
-    LIMIT_AND_OFFSET = 'limit',
-    TAKE_AND_SKIP = 'take',
-}
-
 export interface PaginateConfig<T> {
     relations?: FindOptionsRelations<T> | RelationColumn<T>[]
     sortableColumns: Column<T>[]
@@ -76,13 +71,11 @@ export interface PaginateConfig<T> {
     withDeleted?: boolean
     relativePath?: boolean
     origin?: string
-    paginationType?: PaginationType
 }
 
 export const DEFAULT_MAX_LIMIT = 100
 export const DEFAULT_LIMIT = 20
 export const NO_PAGINATION = 0
-export const DEFAULT_PAGINATE_TYPE = PaginationType.TAKE_AND_SKIP
 
 export async function paginate<T extends ObjectLiteral>(
     query: PaginateQuery,
@@ -94,7 +87,6 @@ export async function paginate<T extends ObjectLiteral>(
     const defaultLimit = config.defaultLimit || DEFAULT_LIMIT
     const maxLimit = positiveNumberOrDefault(config.maxLimit, DEFAULT_MAX_LIMIT)
     const queryLimit = positiveNumberOrDefault(query.limit, defaultLimit)
-    const paginationType = config.paginationType || DEFAULT_PAGINATE_TYPE
 
     const isPaginated = !(queryLimit === NO_PAGINATION && maxLimit === NO_PAGINATION)
 
@@ -151,17 +143,7 @@ export async function paginate<T extends ObjectLiteral>(
     }
 
     if (isPaginated) {
-        // Allow user to choose between limit/offset and take/skip.
-        // However, using limit/offset can return unexpected results.
-        // For more information see:
-        // [#477](https://github.com/ppetzold/nestjs-paginate/issues/477)
-        // [#4742](https://github.com/typeorm/typeorm/issues/4742)
-        // [#5670](https://github.com/typeorm/typeorm/issues/5670)
-        if (paginationType === PaginationType.LIMIT_AND_OFFSET) {
-            queryBuilder.limit(limit).offset((page - 1) * limit)
-        } else {
-            queryBuilder.take(limit).skip((page - 1) * limit)
-        }
+        queryBuilder.limit(limit).offset((page - 1) * limit)
     }
 
     if (config.relations) {