Skip to content

fix::notion-client - Replace ky with got || fix::react-notion-x - LazyImageFull.tsx updated #646

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions examples/full/components/NotionPage.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import dynamic from 'next/dynamic'
import Head from 'next/head'
// import Image from 'next/image'
//import Image from 'next/image'
import Link from 'next/link'
import { useRouter } from 'next/router'
import { type ExtendedRecordMap } from 'notion-types'
Expand Down Expand Up @@ -182,7 +182,7 @@ export function NotionPage({
previewImages={previewImagesEnabled}
components={{
// NOTE (transitive-bullshit 3/12/2023): I'm disabling next/image for this repo for now because the amount of traffic started costing me hundreds of dollars a month in Vercel image optimization costs. I'll probably re-enable it in the future if I can find a better solution.
// nextLegacyImage: Image,
//nextImage: Image,
nextLink: Link,
Code,
Collection,
Expand Down
1 change: 0 additions & 1 deletion examples/full/lib/notion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export async function getPage(pageId: string): Promise<ExtendedRecordMap> {

return recordMap
}

export async function search(params: SearchParams): Promise<SearchResults> {
if ('search' in notion) {
return notion.search(params)
Expand Down
26 changes: 17 additions & 9 deletions examples/full/next.config.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
export default {
staticPageGenerationTimeout: 300,
/** @type {import('next').NextConfig} */
const nextConfig = {
eslint: {
ignoreDuringBuilds: true
},
typescript: {
ignoreBuildErrors: true
},
images: {
domains: [
'www.notion.so',
'notion.so',
'images.unsplash.com',
'abs.twimg.com',
'pbs.twimg.com',
's3.us-west-2.amazonaws.com'
remotePatterns: [
{ protocol: 'https', hostname: 'www.notion.so' },
{ protocol: 'https', hostname: 'notion.so' },
{ protocol: 'https', hostname: 'images.unsplash.com' },
{ protocol: 'https', hostname: 'abs.twimg.com' },
{ protocol: 'https', hostname: 'pbs.twimg.com' },
{ protocol: 'https', hostname: 's3.us-west-2.amazonaws.com' }
],
formats: ['image/avif', 'image/webp']
}
}

export default nextConfig
4 changes: 3 additions & 1 deletion examples/minimal/next.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export default {
const nextConfig = {
staticPageGenerationTimeout: 300,
images: {
domains: [
Expand All @@ -10,3 +10,5 @@ export default {
formats: ['image/avif', 'image/webp']
}
}

export default nextConfig
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"dev": "turbo dev --concurrency 50 --continue --filter='./packages/*'",
"clean": "turbo clean",
"test": "turbo test",
"test:format": "prettier --check \"**/*.{js,ts,tsx}\"",
"test:format": "prettier --check \"**/*.{js,ts,tsx}\" --write",
"test:lint": "turbo test:lint",
"test:typecheck": "turbo test:typecheck",
"test:unit": "turbo test:unit",
Expand Down
4 changes: 2 additions & 2 deletions packages/notion-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@
"watch": "tsup --watch --silent --onSuccess 'echo build successful'",
"clean": "del build",
"test": "run-s test:*",
"test:lint": "eslint .",
"test:lint": "eslint . --fix",
"test:typecheck": "tsc --noEmit",
"test:unit": "vitest run"
},
"dependencies": {
"ky": "catalog:",
"got": "catalog:",
"notion-types": "workspace:*",
"notion-utils": "workspace:*",
"p-map": "catalog:"
Expand Down
54 changes: 23 additions & 31 deletions packages/notion-client/src/notion-api.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// import { promises as fs } from 'fs'
import type * as notion from 'notion-types'
import ky, { type Options as KyOptions } from 'ky'
import got, { type OptionsOfJSONResponseBody as Got_Options } from 'got'
import {
getBlockCollectionId,
getPageContentBlockIds,
Expand All @@ -19,7 +18,7 @@ export class NotionAPI {
private readonly _authToken?: string
private readonly _activeUser?: string
private readonly _userTimeZone: string
private readonly _kyOptions?: KyOptions
private readonly _kyOptions?: Got_Options

constructor({
apiBaseUrl = 'https://www.notion.so/api/v3',
Expand All @@ -33,7 +32,7 @@ export class NotionAPI {
userLocale?: string
userTimeZone?: string
activeUser?: string
kyOptions?: KyOptions
kyOptions?: Got_Options
} = {}) {
this._apiBaseUrl = apiBaseUrl
this._authToken = authToken
Expand Down Expand Up @@ -65,7 +64,7 @@ export class NotionAPI {
throwOnCollectionErrors?: boolean
collectionReducerLimit?: number
fetchRelationPages?: boolean
kyOptions?: KyOptions
kyOptions?: Got_Options
} = {}
): Promise<notion.ExtendedRecordMap> {
const page = await this.getPageRaw(pageId, {
Expand Down Expand Up @@ -232,7 +231,7 @@ export class NotionAPI {

fetchRelationPages = async (
recordMap: notion.ExtendedRecordMap,
kyOptions: KyOptions | undefined
kyOptions: Got_Options | undefined
): Promise<notion.BlockMap> => {
const maxIterations = 10

Expand Down Expand Up @@ -320,7 +319,7 @@ export class NotionAPI {
}: {
recordMap: notion.ExtendedRecordMap
contentBlockIds?: string[]
kyOptions?: KyOptions
kyOptions?: Got_Options
}) {
recordMap.signed_urls = {}

Expand Down Expand Up @@ -401,7 +400,7 @@ export class NotionAPI {
}: {
chunkLimit?: number
chunkNumber?: number
kyOptions?: KyOptions
kyOptions?: Got_Options
} = {}
) {
const parsedPageId = parsePageId(pageId)
Expand Down Expand Up @@ -442,7 +441,7 @@ export class NotionAPI {
userTimeZone?: string
userLocale?: string
loadContentCover?: boolean
kyOptions?: KyOptions
kyOptions?: Got_Options
} = {}
) {
const type = collectionView?.type
Expand Down Expand Up @@ -627,7 +626,9 @@ export class NotionAPI {
loader
},
kyOptions: {
timeout: 60_000,
timeout: {
request: 60_000
},
...kyOptions,
searchParams: {
// TODO: spread kyOptions?.searchParams
Expand All @@ -637,7 +638,7 @@ export class NotionAPI {
})
}

public async getUsers(userIds: string[], kyOptions?: KyOptions) {
public async getUsers(userIds: string[], kyOptions?: Got_Options) {
return this.fetch<notion.RecordValues<notion.User>>({
endpoint: 'getRecordValues',
body: {
Expand All @@ -647,7 +648,7 @@ export class NotionAPI {
})
}

public async getBlocks(blockIds: string[], kyOptions?: KyOptions) {
public async getBlocks(blockIds: string[], kyOptions?: Got_Options) {
return this.fetch<notion.PageChunk>({
endpoint: 'syncRecordValues',
body: {
Expand All @@ -664,7 +665,7 @@ export class NotionAPI {

public async getSignedFileUrls(
urls: types.SignedUrlRequest[],
kyOptions?: KyOptions
kyOptions?: Got_Options
) {
return this.fetch<types.SignedUrlResponse>({
endpoint: 'getSignedFileUrls',
Expand All @@ -675,7 +676,7 @@ export class NotionAPI {
})
}

public async search(params: notion.SearchParams, kyOptions?: KyOptions) {
public async search(params: notion.SearchParams, kyOptions?: Got_Options) {
const body = {
type: 'BlocksInAncestor',
source: 'quick_find_public',
Expand Down Expand Up @@ -715,7 +716,7 @@ export class NotionAPI {
}: {
endpoint: string
body: object
kyOptions?: KyOptions
kyOptions?: Got_Options
headers?: any
}): Promise<T> {
const headers: any = {
Expand All @@ -735,21 +736,12 @@ export class NotionAPI {

const url = `${this._apiBaseUrl}/${endpoint}`

const res = await ky.post(url, {
mode: 'no-cors',
...this._kyOptions,
...kyOptions,
json: body,
headers
})

// TODO: we're awaiting the first fetch and then separately awaiting
// `res.json()` because there seems to be some weird error which repros
// sporadically when loading collections where the body is already used.
// No idea why, but from my testing, separating these into two separate
// steps seems to fix the issue locally for me...
// console.log(endpoint, { bodyUsed: res.bodyUsed })

return res.json<T>()
return got
.post(url, {
...kyOptions,
json: body,
headers
})
.json()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original implementation separated the HTTP request and JSON parsing with a comment noting this was to avoid sporadic errors with body reuse:

// Separate awaits to avoid sporadic errors
const res = await ky.post(url, {...});
return res.json<T>();

The new implementation with got.post().json() combines these steps, which might reintroduce the issue that was previously fixed. Consider maintaining the two-step approach:

const res = await got.post(url, {...});
return res.json();

This preserves the original fix while still using the new HTTP client.

Suggested change
return got
.post(url, {
...kyOptions,
json: body,
headers
})
.json()
const res = await got
.post(url, {
...kyOptions,
json: body,
headers
})
return res.json()

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

}
}
2 changes: 1 addition & 1 deletion packages/notion-client/tsup.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export default defineConfig({
format: ['esm'],
splitting: false,
sourcemap: true,
minify: false,
minify: true,
shims: false,
dts: true
})
2 changes: 1 addition & 1 deletion packages/notion-compat/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"watch": "tsup --watch --silent --onSuccess 'echo build successful'",
"clean": "del build",
"test": "run-s test:*",
"test:lint": "eslint .",
"test:lint": "eslint . --fix",
"test:typecheck": "tsc --noEmit",
"test:unit": "vitest run"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/notion-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"watch": "tsup --watch --silent --onSuccess 'echo build successful'",
"clean": "del build",
"test": "run-s test:*",
"test:lint": "eslint .",
"test:lint": "eslint . --fix",
"test:typecheck": "tsc --noEmit",
"test:unit": "vitest run"
},
Expand Down
13 changes: 7 additions & 6 deletions packages/react-notion-x/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"main": "./build/index.js",
"module": "./build/index.js",
"types": "./build/index.d.ts",
"sideEffects": false,
"sideEffects": ["src/styles.css"],
"exports": {
".": {
"types": "./build/index.d.ts",
Expand Down Expand Up @@ -51,7 +51,7 @@
"watch": "tsup --watch --silent --onSuccess 'echo build successful'",
"clean": "del build",
"test": "run-s test:*",
"test:lint": "eslint .",
"test:lint": "eslint . --fix",
"test:typecheck": "tsc --noEmit"
},
"dependencies": {
Expand All @@ -64,8 +64,9 @@
"react-fast-compare": "catalog:",
"react-hotkeys-hook": "catalog:",
"react-image": "catalog:",
"react-lazy-images": "catalog:",
"react-modal": "catalog:"
"react-intersection-observer": "catalog:",
"react-modal" : "catalog:",
"unionize" : "catalog:"
},
"devDependencies": {
"@types/lodash.throttle": "catalog:",
Expand All @@ -78,13 +79,13 @@
"lodash.throttle": "catalog:",
"react": "catalog:",
"react-dom": "catalog:",
"react-pdf": "^9.1.1"
"react-pdf": "^10.0.1"
},
"peerDependencies": {
"react": ">=18",
"react-dom": ">=18"
},
"optionalDependencies": {
"react-pdf": "^9.2.1"
"react-pdf": "^10.0.1"
}
}
Loading