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

Upgrade support for react 17 #1040

Merged
merged 29 commits into from
Sep 29, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
9593991
[WIP] Upgrade to react 17 support
snowystinger Sep 1, 2020
9d5edcc
remove unneeded chalk patch now that we are using the new version
snowystinger Sep 1, 2020
243e131
fix a ref to the new way of doing things
snowystinger Sep 2, 2020
ee383a7
fix sidenav tests
snowystinger Sep 2, 2020
f144a71
Queue up useId updates to fix bad setState
snowystinger Sep 3, 2020
f3edb0c
Adding a bunch of comments explaining things so that i don't lose my …
snowystinger Sep 3, 2020
319ebc7
make sure to clean up listeners
snowystinger Sep 4, 2020
2b407b8
We can leave these not capturing for now, no tests are failing currently
snowystinger Sep 4, 2020
3b3d8d6
one more comment
snowystinger Sep 4, 2020
a6e41f8
Trying to find the node error stack
snowystinger Sep 4, 2020
fd8d1be
Fixes the remainder of the tests, however, i need to add one now for …
snowystinger Sep 4, 2020
e46728e
put us back on react 16, with allowance for 17, i'll add circlci job …
snowystinger Sep 4, 2020
6a72b6f
reduce updates and fix yarn lock
snowystinger Sep 4, 2020
55cc73f
Got a working scenario for 17
snowystinger Sep 5, 2020
6a569fc
Fix up useId so that it runs immediately if it's not in a render cycle
snowystinger Sep 5, 2020
85ce3a8
fix lint
snowystinger Sep 5, 2020
16052a4
Adding test
snowystinger Sep 14, 2020
fdf17b9
can not put NODE_ENV before the yarn add command, it causes icons to …
snowystinger Sep 14, 2020
4576e75
correct range dep for root package
snowystinger Sep 16, 2020
75409ce
fix yarn lock
snowystinger Sep 16, 2020
9ccf2cf
Increase timeout time so we can pass our tests. Will make issue to ad…
snowystinger Sep 22, 2020
2e1d920
remove unneeded chalk patch now that we are using the new version
snowystinger Sep 23, 2020
3f00c58
remove stale comment
snowystinger Sep 23, 2020
cebffd5
remove more stale comments
snowystinger Sep 23, 2020
4c59490
Merge branch 'main' into pass-react-17-in-jest
dannify Sep 25, 2020
481dbc8
Merge branch 'main' into pass-react-17-in-jest
snowystinger Sep 25, 2020
b70e050
fix syntax error from merge
snowystinger Sep 25, 2020
ed0f7a1
Merge branch 'main' into pass-react-17-in-jest
devongovett Sep 29, 2020
6a00b91
Merge branch 'main' into pass-react-17-in-jest
devongovett Sep 29, 2020
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
Prev Previous commit
Next Next commit
Fix up useId so that it runs immediately if it's not in a render cycle
update comments
  • Loading branch information
snowystinger committed Sep 23, 2020
commit 6a569fc2d1b4900b7f2bee54a2ff37b8125e0543
5 changes: 5 additions & 0 deletions packages/@react-aria/focus/src/FocusScope.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,11 @@ function useFocusContainment(scopeRef: RefObject<HTMLElement[]>, contain: boolea
}
};

/**
* Need to add back in the stop propagation in some form.
* Need a test to make sure that the outer scope doesn't restore focus when there's an inner scope already.
*/

document.addEventListener('keydown', onKeyDown, false);
document.addEventListener('focusin', onFocus, false);
scope.forEach(element => element.addEventListener('focusin', onFocus, false));
Expand Down
31 changes: 18 additions & 13 deletions packages/@react-aria/utils/src/useId.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,30 @@ let map: Map<string, (v: string) => void> = new Map();
* @param defaultId - Default component id.
*/
export function useId(defaultId?: string): string {
let isRendering = useRef(true);
isRendering.current = true;
let [value, setValue] = useState(defaultId);
// Do i need to keep an entire queue? i think I only need the most recent?
// Does this actually work in the case that we're not mid render? It's a ref, so it won't
// cause a re-render ever on it's own, that said, we might always be mid-render
// we seem to only call mergeProps/Ids in the render flow, but now we'll need to document that
// as a limitation of those functions.
let setIdUpdateQueue = useRef([]);
let pushToQueue = useCallback((val) => {
setIdUpdateQueue.current.push(val);
}, [setIdUpdateQueue.current]);
let nextId = useRef(null);
// don't memo this, we want it new each render so that the Effects always run
let updateValue = (val) => {
if (!isRendering.current) {
setValue(val);
} else {
nextId.current = val;
}
};
useLayoutEffect(() => {
isRendering.current = false;
}, [updateValue])
useEffect(() => {
let newId = setIdUpdateQueue.current.pop();
let newId = nextId.current;
if (newId) {
setValue(newId);
nextId.current = null;
}
setIdUpdateQueue.current = [];
}, [setValue, pushToQueue]);
}, [setValue, updateValue]);
let res = useSSRSafeId(value);
map.set(res, pushToQueue);
map.set(res, updateValue);
return res;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/@react-spectrum/dialog/test/DialogTrigger.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,6 @@ describe('DialogTrigger', function () {
expect(tray).toBeVisible();
});

// see notes in focus scope, fails in .only mode as well
it('should restore focus to the trigger when the dialog is closed', async function () {
let {getByRole} = render(
<Provider theme={theme}>
Expand Down Expand Up @@ -265,6 +264,7 @@ describe('DialogTrigger', function () {
expect(dialog).not.toBeInTheDocument();
}); // wait for animation

// now that it's been unmounted, run the raf callback
act(() => {
jest.runAllTimers();
});
Expand Down