Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit156f985

Browse files
authored
fix(site): updateuseClipboard to work better with effect logic (#20183)
## Changes made- Updated `useClipboard` API to require passing the text in via the`copyToClipboard` function, rather than requiring that the text getsspecified in render logic- Ensured that the `copyToClipboard` function always stays stable acrossall React lifecycles- Updated all existing uses to use the new function signatures- Updated all tests and added new cases
1 parent09f69ef commit156f985

File tree

9 files changed

+173
-87
lines changed

9 files changed

+173
-87
lines changed

‎site/src/components/CopyButton/CopyButton.tsx‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,7 @@ export const CopyButton: FC<CopyButtonProps> = ({
1919
label,
2020
...buttonProps
2121
})=>{
22-
const{ showCopiedSuccess, copyToClipboard}=useClipboard({
23-
textToCopy:text,
24-
});
22+
const{ showCopiedSuccess, copyToClipboard}=useClipboard();
2523

2624
return(
2725
<TooltipProvider>
@@ -30,7 +28,7 @@ export const CopyButton: FC<CopyButtonProps> = ({
3028
<Button
3129
size="icon"
3230
variant="subtle"
33-
onClick={copyToClipboard}
31+
onClick={()=>copyToClipboard(text)}
3432
{...buttonProps}
3533
>
3634
{showCopiedSuccess ?<CheckIcon/> :<CopyIcon/>}

‎site/src/components/CopyableValue/CopyableValue.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ export const CopyableValue: FC<CopyableValueProps> = ({
1616
children,
1717
...attrs
1818
})=>{
19-
const{ showCopiedSuccess, copyToClipboard}=useClipboard({
20-
textToCopy:value,
19+
const{ showCopiedSuccess, copyToClipboard}=useClipboard();
20+
constclickableProps=useClickable<HTMLSpanElement>(()=>{
21+
copyToClipboard(value);
2122
});
22-
constclickableProps=useClickable<HTMLSpanElement>(copyToClipboard);
2323

2424
return(
2525
<Tooltip

‎site/src/hooks/useClickable.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
typeRefObject,
55
useRef,
66
}from"react";
7+
import{useEffectEvent}from"./hookPolyfills";
78

89
// Literally any object (ideally an HTMLElement) that has a .click method
910
typeClickableElement={
@@ -43,10 +44,11 @@ export const useClickable = <
4344
role?:TRole,
4445
):UseClickableResult<TElement,TRole>=>{
4546
constref=useRef<TElement>(null);
47+
conststableOnClick=useEffectEvent(onClick);
4648

4749
return{
4850
ref,
49-
onClick,
51+
onClick:stableOnClick,
5052
tabIndex:0,
5153
role:(role??"button")asTRole,
5254

‎site/src/hooks/useClipboard.test.tsx‎

Lines changed: 96 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@
99
* immediately pollutes the tests with false negatives. Even if something should
1010
* fail, it won't.
1111
*/
12-
import{act,renderHook,screen}from"@testing-library/react";
12+
13+
import{renderHook,screen}from"@testing-library/react";
1314
import{GlobalSnackbar}from"components/GlobalSnackbar/GlobalSnackbar";
1415
import{ThemeOverride}from"contexts/ThemeProvider";
16+
import{act}from"react";
1517
importthemes,{DEFAULT_THEME}from"theme";
1618
import{
1719
COPY_FAILED_MESSAGE,
@@ -115,8 +117,8 @@ function setupMockClipboard(isSecure: boolean): SetupMockClipboardResult {
115117
};
116118
}
117119

118-
functionrenderUseClipboard<TInputextendsUseClipboardInput>(inputs:TInput){
119-
returnrenderHook<UseClipboardResult,TInput>(
120+
functionrenderUseClipboard(inputs?:UseClipboardInput){
121+
returnrenderHook<UseClipboardResult,UseClipboardInput>(
120122
(props)=>useClipboard(props),
121123
{
122124
initialProps:inputs,
@@ -188,9 +190,9 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
188190

189191
constassertClipboardUpdateLifecycle=async(
190192
result:RenderResult,
191-
textToCheck:string,
193+
textToCopy:string,
192194
):Promise<void>=>{
193-
awaitact(()=>result.current.copyToClipboard());
195+
awaitact(()=>result.current.copyToClipboard(textToCopy));
194196
expect(result.current.showCopiedSuccess).toBe(true);
195197

196198
// Because of timing trickery, any timeouts for flipping the copy status
@@ -203,35 +205,35 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
203205
awaitact(()=>jest.runAllTimersAsync());
204206

205207
constclipboardText=getClipboardText();
206-
expect(clipboardText).toEqual(textToCheck);
208+
expect(clipboardText).toEqual(textToCopy);
207209
};
208210

209211
it("Copies the current text to the user's clipboard",async()=>{
210212
consttextToCopy="dogs";
211-
const{ result}=renderUseClipboard({ textToCopy});
213+
const{ result}=renderUseClipboard();
212214
awaitassertClipboardUpdateLifecycle(result,textToCopy);
213215
});
214216

215217
it("Should indicate to components not to show successful copy after a set period of time",async()=>{
216218
consttextToCopy="cats";
217-
const{ result}=renderUseClipboard({ textToCopy});
219+
const{ result}=renderUseClipboard();
218220
awaitassertClipboardUpdateLifecycle(result,textToCopy);
219221
expect(result.current.showCopiedSuccess).toBe(false);
220222
});
221223

222224
it("Should notify the user of an error using the provided callback",async()=>{
223225
consttextToCopy="birds";
224226
constonError=jest.fn();
225-
const{ result}=renderUseClipboard({textToCopy,onError});
227+
const{ result}=renderUseClipboard({ onError});
226228

227229
setSimulateFailure(true);
228-
awaitact(()=>result.current.copyToClipboard());
230+
awaitact(()=>result.current.copyToClipboard(textToCopy));
229231
expect(onError).toBeCalled();
230232
});
231233

232234
it("Should dispatch a new toast message to the global snackbar when errors happen while no error callback is provided to the hook",async()=>{
233235
consttextToCopy="crow";
234-
const{ result}=renderUseClipboard({ textToCopy});
236+
const{ result}=renderUseClipboard();
235237

236238
/**
237239
*@todo Look into why deferring error-based state updates to the global
@@ -241,7 +243,7 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
241243
* flushed through the GlobalSnackbar component afterwards
242244
*/
243245
setSimulateFailure(true);
244-
awaitact(()=>result.current.copyToClipboard());
246+
awaitact(()=>result.current.copyToClipboard(textToCopy));
245247

246248
consterrorMessageNode=screen.queryByText(COPY_FAILED_MESSAGE);
247249
expect(errorMessageNode).not.toBeNull();
@@ -252,11 +254,91 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
252254
// Snackbar state transitions that you might get if the hook uses the
253255
// default
254256
consttextToCopy="hamster";
255-
const{ result}=renderUseClipboard({ textToCopy,onError:jest.fn()});
257+
const{ result}=renderUseClipboard({onError:jest.fn()});
258+
259+
setSimulateFailure(true);
260+
awaitact(()=>result.current.copyToClipboard(textToCopy));
261+
262+
expect(result.current.error).toBeInstanceOf(Error);
263+
});
256264

265+
it("Clears out existing errors if a new copy operation succeeds",async()=>{
266+
consttext="dummy-text";
267+
const{ result}=renderUseClipboard();
257268
setSimulateFailure(true);
258-
awaitact(()=>result.current.copyToClipboard());
259269

270+
awaitact(()=>result.current.copyToClipboard(text));
260271
expect(result.current.error).toBeInstanceOf(Error);
272+
273+
setSimulateFailure(false);
274+
awaitassertClipboardUpdateLifecycle(result,text);
275+
expect(result.current.error).toBeUndefined();
276+
});
277+
278+
// This test case is really important to ensure that it's easy to plop this
279+
// inside of useEffect calls without having to think about dependencies too
280+
// much
281+
it("Ensures that the copyToClipboard function always maintains a stable reference across all re-renders",async()=>{
282+
constinitialOnError=jest.fn();
283+
const{ result, rerender}=renderUseClipboard({
284+
onError:initialOnError,
285+
clearErrorOnSuccess:true,
286+
});
287+
constinitialCopy=result.current.copyToClipboard;
288+
289+
// Re-render arbitrarily with no clipboard state transitions to make
290+
// sure that a parent re-rendering doesn't break anything
291+
rerender({onError:initialOnError});
292+
expect(result.current.copyToClipboard).toBe(initialCopy);
293+
294+
// Re-render with new onError prop and then swap back to simplify
295+
// testing
296+
rerender({onError:jest.fn()});
297+
expect(result.current.copyToClipboard).toBe(initialCopy);
298+
rerender({onError:initialOnError});
299+
300+
// Re-render with a new clear value then swap back to simplify testing
301+
rerender({onError:initialOnError,clearErrorOnSuccess:false});
302+
expect(result.current.copyToClipboard).toBe(initialCopy);
303+
rerender({onError:initialOnError,clearErrorOnSuccess:true});
304+
305+
// Trigger a failed clipboard interaction
306+
setSimulateFailure(true);
307+
awaitact(()=>result.current.copyToClipboard("dummy-text-2"));
308+
expect(result.current.copyToClipboard).toBe(initialCopy);
309+
310+
/**
311+
* Trigger a successful clipboard interaction
312+
*
313+
*@todo For some reason, using the assertClipboardUpdateLifecycle
314+
* helper triggers Jest errors with it thinking that values are being
315+
* accessed after teardown, even though the problem doesn't exist for
316+
* any other test case.
317+
*
318+
* It's not a huge deal, because we only need to inspect React after the
319+
* interaction, instead of the full DOM, but for correctness, it would
320+
* be nice if we could get this issue figured out.
321+
*/
322+
setSimulateFailure(false);
323+
awaitact(()=>result.current.copyToClipboard("dummy-text-2"));
324+
expect(result.current.copyToClipboard).toBe(initialCopy);
325+
});
326+
327+
it("Always uses the most up-to-date onError prop",async()=>{
328+
constinitialOnError=jest.fn();
329+
const{ result, rerender}=renderUseClipboard({
330+
onError:initialOnError,
331+
});
332+
setSimulateFailure(true);
333+
334+
constsecondOnError=jest.fn();
335+
rerender({onError:secondOnError});
336+
awaitact(()=>result.current.copyToClipboard("dummy-text"));
337+
338+
expect(initialOnError).not.toHaveBeenCalled();
339+
expect(secondOnError).toHaveBeenCalledTimes(1);
340+
expect(secondOnError).toHaveBeenCalledWith(
341+
"Failed to copy text to clipboard",
342+
);
261343
});
262344
});

‎site/src/hooks/useClipboard.ts‎

Lines changed: 41 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,18 @@
11
import{displayError}from"components/GlobalSnackbar/utils";
2-
import{useEffect,useRef,useState}from"react";
2+
import{useCallback,useEffect,useRef,useState}from"react";
3+
import{useEffectEvent}from"./hookPolyfills";
34

45
constCLIPBOARD_TIMEOUT_MS=1_000;
56
exportconstCOPY_FAILED_MESSAGE="Failed to copy text to clipboard";
67
exportconstHTTP_FALLBACK_DATA_ID="http-fallback";
78

89
exporttypeUseClipboardInput=Readonly<{
9-
textToCopy:string;
10-
11-
/**
12-
* Optional callback to call when an error happens. If not specified, the hook
13-
* will dispatch an error message to the GlobalSnackbar
14-
*/
1510
onError?:(errorMessage:string)=>void;
11+
clearErrorOnSuccess?:boolean;
1612
}>;
1713

1814
exporttypeUseClipboardResult=Readonly<{
19-
copyToClipboard:()=>Promise<void>;
15+
copyToClipboard:(textToCopy:string)=>Promise<void>;
2016
error:Error|undefined;
2117

2218
/**
@@ -40,47 +36,56 @@ export type UseClipboardResult = Readonly<{
4036
showCopiedSuccess:boolean;
4137
}>;
4238

43-
exportconstuseClipboard=(input:UseClipboardInput):UseClipboardResult=>{
44-
const{ textToCopy,onError:errorCallback}=input;
39+
exportconstuseClipboard=(input?:UseClipboardInput):UseClipboardResult=>{
40+
const{ onError=displayError, clearErrorOnSuccess=true}=input??{};
41+
4542
const[showCopiedSuccess,setShowCopiedSuccess]=useState(false);
4643
const[error,setError]=useState<Error>();
4744
consttimeoutIdRef=useRef<number|undefined>(undefined);
4845

4946
useEffect(()=>{
50-
constclearIdOnUnmount=()=>window.clearTimeout(timeoutIdRef.current);
51-
returnclearIdOnUnmount;
47+
constclearTimeoutOnUnmount=()=>{
48+
window.clearTimeout(timeoutIdRef.current);
49+
};
50+
returnclearTimeoutOnUnmount;
5251
},[]);
5352

54-
consthandleSuccessfulCopy=()=>{
53+
conststableOnError=useEffectEvent(()=>onError(COPY_FAILED_MESSAGE));
54+
consthandleSuccessfulCopy=useEffectEvent(()=>{
5555
setShowCopiedSuccess(true);
56+
if(clearErrorOnSuccess){
57+
setError(undefined);
58+
}
59+
5660
timeoutIdRef.current=window.setTimeout(()=>{
5761
setShowCopiedSuccess(false);
5862
},CLIPBOARD_TIMEOUT_MS);
59-
};
60-
61-
constcopyToClipboard=async()=>{
62-
try{
63-
awaitwindow.navigator.clipboard.writeText(textToCopy);
64-
handleSuccessfulCopy();
65-
}catch(err){
66-
constfallbackCopySuccessful=simulateClipboardWrite(textToCopy);
67-
if(fallbackCopySuccessful){
68-
handleSuccessfulCopy();
69-
return;
70-
}
63+
});
7164

72-
constwrappedErr=newError(COPY_FAILED_MESSAGE);
73-
if(errinstanceofError){
74-
wrappedErr.stack=err.stack;
65+
constcopyToClipboard=useCallback(
66+
async(textToCopy:string)=>{
67+
try{
68+
awaitwindow.navigator.clipboard.writeText(textToCopy);
69+
handleSuccessfulCopy();
70+
}catch(err){
71+
constfallbackCopySuccessful=simulateClipboardWrite(textToCopy);
72+
if(fallbackCopySuccessful){
73+
handleSuccessfulCopy();
74+
return;
75+
}
76+
77+
constwrappedErr=newError(COPY_FAILED_MESSAGE);
78+
if(errinstanceofError){
79+
wrappedErr.stack=err.stack;
80+
}
81+
82+
console.error(wrappedErr);
83+
setError(wrappedErr);
84+
stableOnError();
7585
}
76-
77-
console.error(wrappedErr);
78-
setError(wrappedErr);
79-
80-
constnotifyUser=errorCallback??displayError;
81-
notifyUser(COPY_FAILED_MESSAGE);
82-
}
83-
};
86+
},
87+
[stableOnError,handleSuccessfulCopy],
88+
);
8489

8590
return{ showCopiedSuccess, error, copyToClipboard};
8691
};

‎site/src/pages/CliAuthPage/CliAuthPageView.tsx‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,7 @@ interface CliAuthPageViewProps {
1212
}
1313

1414
exportconstCliAuthPageView:FC<CliAuthPageViewProps>=({ sessionToken})=>{
15-
constclipboard=useClipboard({
16-
textToCopy:sessionToken??"",
17-
});
18-
15+
constclipboardState=useClipboard();
1916
return(
2017
<SignInLayout>
2118
<Welcome>Session token</Welcome>
@@ -30,16 +27,20 @@ export const CliAuthPageView: FC<CliAuthPageViewProps> = ({ sessionToken }) => {
3027
className="w-full"
3128
size="lg"
3229
disabled={!sessionToken}
33-
onClick={clipboard.copyToClipboard}
30+
onClick={()=>{
31+
if(sessionToken){
32+
clipboardState.copyToClipboard(sessionToken);
33+
}
34+
}}
3435
>
35-
{clipboard.showCopiedSuccess ?(
36+
{clipboardState.showCopiedSuccess ?(
3637
<CheckIcon/>
3738
) :(
3839
<Spinnerloading={!sessionToken}>
3940
<CopyIcon/>
4041
</Spinner>
4142
)}
42-
{clipboard.showCopiedSuccess
43+
{clipboardState.showCopiedSuccess
4344
?"Session token copied!"
4445
:"Copy session token"}
4546
</Button>

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp