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

Replaced third-party lazy-loading implementation with native lazy-loading for improved performance #602

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion examples/full/next-env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
/// <reference types="next/image-types/global" />

// NOTE: This file should not be edited
// see https://nextjs.org/docs/pages/building-your-application/configuring/typescript for more information.
// see https://nextjs.org/docs/pages/api-reference/config/typescript for more information.
2 changes: 1 addition & 1 deletion examples/minimal/next-env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
/// <reference types="next/image-types/global" />

// NOTE: This file should not be edited
// see https://nextjs.org/docs/pages/building-your-application/configuring/typescript for more information.
// see https://nextjs.org/docs/pages/api-reference/config/typescript for more information.
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,16 @@
"del-cli": "^6.0.0",
"eslint": "^8.57.1",
"npm-run-all2": "^7.0.1",
"prettier": "^3.3.3",
"prettier": "^3.4.2",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"tsup": "^8.3.5",
"tsx": "^4.19.2",
"turbo": "^2.3.0",
"typescript": "^5.6.3",
"vitest": "^2.1.5"
},
"dependencies": {
"@types/react": "^18.3.12"
}
}
7 changes: 5 additions & 2 deletions packages/notion-client/src/notion-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,14 +237,17 @@ export class NotionAPI {
// console.log(block, source)

if (source) {
if (source.includes('secure.notion-static.com') || source.includes('prod-files-secure')) {
if (
source.includes('secure.notion-static.com') ||
source.includes('prod-files-secure')
) {
return {
permissionRecord: {
table: 'block',
id: block.id
},
url: source
};
}
}

return []
Expand Down
1 change: 0 additions & 1 deletion packages/react-notion-x/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@
"react-fast-compare": "^3.2.0",
"react-hotkeys-hook": "^4.5.1",
"react-image": "^4.0.3",
"react-lazy-images": "^1.1.0",
"react-modal": "^3.14.3"
},
"devDependencies": {
Expand Down
177 changes: 11 additions & 166 deletions packages/react-notion-x/src/components/lazy-image.tsx
Original file line number Diff line number Diff line change
@@ -1,182 +1,27 @@
import { normalizeUrl } from 'notion-utils'
import * as React from 'react'
import { ImageState, LazyImageFull } from 'react-lazy-images'

import { useNotionContext } from '../context'
import { cs } from '../utils'

/**
* Progressive, lazy images modeled after Medium's LQIP technique.
*/
export function LazyImage({
src,
alt,
className,
style,
zoomable = false,
priority = false,
height,
...rest
priority = false
}: {
src?: string
alt?: string
className?: string
style?: React.CSSProperties
height?: number
zoomable?: boolean
priority?: boolean
}) {
const { recordMap, zoom, previewImages, forceCustomImages, components } =
useNotionContext()
const zoomRef = React.useRef(zoom ? zoom.clone() : null)
const previewImage = previewImages
? (recordMap?.preview_images?.[src!] ??
recordMap?.preview_images?.[normalizeUrl(src)])
: null

const onLoad = React.useCallback(
(e: any) => {
if (zoomable && (e.target.src || e.target.srcset)) {
if (zoomRef.current) {
;(zoomRef.current as any).attach(e.target)
}
}
},
[zoomRef, zoomable]
return (
<img
src={src}
alt={alt}
className={className}
style={style}
height={height}
loading={priority ? 'eager' : 'lazy'} // Native lazy loading
decoding='async' // For better performance and asynchronous decoding
/>
)

const attachZoom = React.useCallback(
(image: any) => {
if (zoomRef.current && image) {
;(zoomRef.current as any).attach(image)
}
},
[zoomRef]
)

const attachZoomRef = React.useMemo(
() => (zoomable ? attachZoom : undefined),
[zoomable, attachZoom]
)

if (previewImage) {
const aspectRatio = previewImage.originalHeight / previewImage.originalWidth

if (components.Image) {
// TODO: could try using next/image onLoadComplete to replace LazyImageFull
// while retaining our blur implementation
return (
<components.Image
src={src}
alt={alt}
style={style}
className={className}
width={previewImage.originalWidth}
height={previewImage.originalHeight}
blurDataURL={previewImage.dataURIBase64}
placeholder='blur'
priority={priority}
onLoad={onLoad}
/>
)
}

return (
// @ts-expect-error LazyImage types are out-of-date.
<LazyImageFull src={src!} {...rest} experimentalDecode={true}>
{({ imageState, ref }) => {
const isLoaded = imageState === ImageState.LoadSuccess
const wrapperStyle: React.CSSProperties = {
width: '100%'
}
const imgStyle: React.CSSProperties = {}

if (height) {
wrapperStyle.height = height
} else {
imgStyle.position = 'absolute'
wrapperStyle.paddingBottom = `${aspectRatio * 100}%`
}

return (
<div
className={cs(
'lazy-image-wrapper',
isLoaded && 'lazy-image-loaded',
className
)}
style={wrapperStyle}
>
<img
className='lazy-image-preview'
src={previewImage.dataURIBase64}
alt={alt}
ref={ref}
style={style}
decoding='async'
/>

<img
className='lazy-image-real'
src={src}
alt={alt}
ref={attachZoomRef}
style={{
...style,
...imgStyle
}}
width={previewImage.originalWidth}
height={previewImage.originalHeight}
decoding='async'
loading='lazy'
/>
</div>
)
}}
</LazyImageFull>
)
} else {
// TODO: GracefulImage doesn't seem to support refs, but we'd like to prevent
// invalid images from loading as error states

/*
NOTE: Using next/image without a pre-defined width/height is a huge pain in
the ass. If we have a preview image, then this works fine since we know the
dimensions ahead of time, but if we don't, then next/image won't display
anything.

Since next/image is the most common use case for using custom images, and this
is likely to trip people up, we're disabling non-preview custom images for now.

If you have a use case that is affected by this, please open an issue on github.
*/
if (components.Image && forceCustomImages) {
return (
<components.Image
src={src}
alt={alt}
className={className}
style={style}
width={null}
height={height || null}
priority={priority}
onLoad={onLoad}
/>
)
}

// Default image element
return (
<img
className={className}
style={style}
src={src}
alt={alt}
ref={attachZoomRef}
loading='lazy'
decoding='async'
{...rest}
/>
)
}
}
Loading