Skip to content

Commit f92888c

Browse files
fix: prevent mapped lists from creating displayName. (#6)
1 parent 2ad49b7 commit f92888c

6 files changed

Lines changed: 168 additions & 6 deletions

File tree

eslint.config.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export default tseslint.config(
2525
files: ['test/fixtures/**/*.tsx'],
2626
rules: {
2727
'@typescript-eslint/no-unused-vars': 'off',
28+
'n/no-missing-import': 'off',
2829
},
2930
},
3031
{

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@knighted/display-name",
3-
"version": "1.0.0-alpha.3",
3+
"version": "1.0.0-rc.0",
44
"description": "Codemod to add React displayName properties to function components.",
55
"type": "module",
66
"exports": {

src/displayName.ts

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,13 @@ import { readFile } from 'node:fs/promises'
22

33
import MagicString from 'magic-string'
44
import { asyncAncestorWalk, ancestorWalk, walk } from '@knighted/walk'
5-
import { parseSync, type Node, type FunctionBody, type Expression } from 'oxc-parser'
5+
import {
6+
parseSync,
7+
type Node,
8+
type FunctionBody,
9+
type Expression,
10+
type VariableDeclarator,
11+
} from 'oxc-parser'
612

713
type Options = {
814
requirePascal?: boolean
@@ -55,6 +61,30 @@ const hasJsx = async (body: FunctionBody | Expression) => {
5561

5662
return found
5763
}
64+
/**
65+
* Useful for preventing mapped JSX lists inside functions from creating
66+
* a displayName when inside a named function.
67+
*
68+
* A simpler fix would be to add the found named function to `foundDisplayNames`
69+
* but that would prevent reusing a displayName for named functions
70+
* (which seems like a bad practice overall).
71+
*/
72+
const createsNamedReactFunction = (declarator: VariableDeclarator) => {
73+
if (declarator.init) {
74+
if (declarator.init.type === 'FunctionExpression' && declarator.init.id) {
75+
return true
76+
}
77+
78+
if (
79+
declarator.init.type === 'CallExpression' &&
80+
declarator.init.arguments.some(arg => arg.type === 'FunctionExpression' && arg.id)
81+
) {
82+
return true
83+
}
84+
}
85+
86+
return false
87+
}
5888
const defaultOptions = {
5989
requirePascal: true,
6090
insertSemicolon: true,
@@ -75,8 +105,9 @@ const modify = async (source: string, options: Options = defaultOptions) => {
75105
case 'ArrowFunctionExpression':
76106
{
77107
const { body, id } = node
108+
const isReact = !!body && (await hasJsx(body))
78109

79-
if (!id && body && (await hasJsx(body))) {
110+
if (!id && isReact) {
80111
/**
81112
* If the function expression is not named,
82113
* use the varible name to set the displayName.
@@ -91,7 +122,8 @@ const modify = async (source: string, options: Options = defaultOptions) => {
91122
if (
92123
declarator.type === 'VariableDeclarator' &&
93124
declarator.id.type === 'Identifier' &&
94-
!foundDisplayNames.includes(declarator.id.name)
125+
!foundDisplayNames.includes(declarator.id.name) &&
126+
!createsNamedReactFunction(declarator)
95127
) {
96128
const { name } = declarator.id
97129
const declaration = ancestors[declaratorIndex - 1]

test/displayName.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,4 +425,12 @@ describe('@knighted/displayName', () => {
425425
`.replace(/\s/g, ''),
426426
)
427427
})
428+
429+
it('works with retyped memo, generics and named function expressions', async () => {
430+
const file = resolve(import.meta.dirname, './fixtures/typed.tsx')
431+
const code = await modifyFile(file)
432+
433+
assert.ok(code.indexOf("Items.displayName = 'Items'") === -1)
434+
assert.ok(code.indexOf('displayName') === -1)
435+
})
428436
})

test/fixtures/typed.tsx

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
import type { ReactNode } from 'react'
2+
import React, { useRef, useLayoutEffect, useState } from 'react'
3+
import { typedMemo } from './fake.js'
4+
import { useMultiComboBoxContext } from './fake'
5+
import { List, Item } from './fake.js'
6+
7+
type ItemsProps<T> = {
8+
children?: (props: { item: T; index: number; isSelected: boolean }) => ReactNode
9+
}
10+
11+
const Other = function Other(props) {
12+
return (
13+
<ul>
14+
{props.items.map(item => {
15+
;<li>{item}</li>
16+
})}
17+
</ul>
18+
)
19+
}
20+
const Items = typedMemo(function Items<T>({ children }: ItemsProps<T>) {
21+
const {
22+
getMenuProps,
23+
getItemProps,
24+
filteredItems,
25+
customItemsList,
26+
selectedItems,
27+
itemToString,
28+
setItemsList,
29+
loadItems,
30+
inputValue,
31+
isOpen,
32+
} = useMultiComboBoxContext<T>()
33+
const customLength = customItemsList.length
34+
const ref = useRef<HTMLUListElement>(null)
35+
const [hasMore, setHasMore] = useState(true)
36+
37+
useLayoutEffect(() => {
38+
if (ref.current && isOpen && loadItems) {
39+
const root = ref.current
40+
const target = root.querySelector(':last-child')
41+
const rootMargin = target ? `${target.clientHeight}px 0px 0px 0px` : '0px'
42+
const observer = new IntersectionObserver(
43+
async entries => {
44+
for (const entry of entries) {
45+
if (entry.isIntersecting && hasMore) {
46+
const { items, hasMore } = await loadItems(inputValue)
47+
48+
setHasMore(hasMore)
49+
setItemsList(prevItems => Array.from(new Set([...prevItems, ...items])))
50+
}
51+
}
52+
},
53+
{
54+
root,
55+
rootMargin,
56+
threshold: 0,
57+
},
58+
)
59+
60+
if (target) {
61+
observer.observe(target)
62+
}
63+
64+
return () => {
65+
observer.disconnect()
66+
}
67+
}
68+
}, [
69+
ref,
70+
isOpen,
71+
// Create a new observer after filtering items
72+
inputValue,
73+
filteredItems,
74+
hasMore,
75+
setItemsList,
76+
loadItems,
77+
])
78+
79+
return (
80+
<List {...getMenuProps({ ref })}>
81+
{isOpen &&
82+
customItemsList.map((item, index) => (
83+
<Item
84+
isLastCustom={index === customItemsList.length - 1}
85+
isSelected={selectedItems.some(
86+
selectedItem => itemToString(selectedItem) === item,
87+
)}
88+
key={`${item}-${index}`}
89+
{...getItemProps({
90+
item,
91+
index,
92+
})}
93+
>
94+
{itemToString(item)}
95+
</Item>
96+
))}
97+
{isOpen &&
98+
filteredItems.map((item, index) => {
99+
const isSelected = selectedItems.some(
100+
selectedItem => itemToString(selectedItem) === itemToString(item),
101+
)
102+
103+
return (
104+
<Item
105+
isSelected={isSelected}
106+
key={`${itemToString(item)}-${index}`}
107+
{...getItemProps({
108+
item,
109+
index: customLength + index,
110+
})}
111+
>
112+
{children ? children({ item, index, isSelected }) : itemToString(item)}
113+
</Item>
114+
)
115+
})}
116+
</List>
117+
)
118+
})
119+
120+
export type { ItemsProps }
121+
export { Items, Other }

0 commit comments

Comments
 (0)