Skip to content

Commit

Permalink
Adapt to useRef type changes
Browse files Browse the repository at this point in the history
`useRef(null as T|null)` now returns a `{ current: T|null }` instead of
`{ current: T }` as it did before. ie. it no longer drops the the null.
This makes sense but conflicted with a pattern we used in many places to
create a non-null ref: `useRef(/** @type {T|null} */ (null))`.

Resolve this by changing all non-nullable refs, for elements which are
set after the initial render, to cast the `useRef` result instead of the
init value.

```
const nonNullRef = /** @type {{ current: T }} */ (useRef());
```
  • Loading branch information
robertknight committed Nov 11, 2021
1 parent 2bb54dc commit 8ee4cec
Show file tree
Hide file tree
Showing 13 changed files with 33 additions and 31 deletions.
26 changes: 12 additions & 14 deletions src/annotator/components/NotebookModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ function NotebookIframe({ config, groupId }) {
);
}

/** @typedef {import('../util/emitter').Emitter} Emitter */

/**
* @typedef NotebookModalProps
* @prop {import('../util/emitter').EventBus} eventBus
Expand All @@ -53,9 +55,7 @@ export default function NotebookModal({ eventBus, config }) {
const [isHidden, setIsHidden] = useState(true);
const [groupId, setGroupId] = useState(/** @type {string|null} */ (null));
const originalDocumentOverflowStyle = useRef('');
const emitter = useRef(
/** @type {ReturnType<eventBus['createEmitter']>|null} */ (null)
);
const emitterRef = useRef(/** @type {Emitter|null} */ (null));

// Stores the original overflow CSS property of document.body and reset it
// when the component is destroyed
Expand All @@ -78,24 +78,22 @@ export default function NotebookModal({ eventBus, config }) {
}, [isHidden]);

useEffect(() => {
emitter.current = eventBus.createEmitter();
emitter.current.subscribe(
'openNotebook',
(/** @type {string} */ groupId) => {
setIsHidden(false);
setIframeKey(iframeKey => iframeKey + 1);
setGroupId(groupId);
}
);
const emitter = eventBus.createEmitter();
emitter.subscribe('openNotebook', (/** @type {string} */ groupId) => {
setIsHidden(false);
setIframeKey(iframeKey => iframeKey + 1);
setGroupId(groupId);
});
emitterRef.current = emitter;

return () => {
emitter.current.destroy();
emitter.destroy();
};
}, [eventBus]);

const onClose = () => {
setIsHidden(true);
emitter.current.publish('closeNotebook');
emitterRef.current?.publish('closeNotebook');
};

if (groupId === null) {
Expand Down
2 changes: 1 addition & 1 deletion src/annotator/util/emitter.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { TinyEmitter } from 'tiny-emitter';
*
* @implements Destroyable
*/
class Emitter {
export class Emitter {
/**
* @param {TinyEmitter} emitter
*/
Expand Down
3 changes: 3 additions & 0 deletions src/sidebar/components/Annotation/AnnotationShareControl.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ function AnnotationShareControl({
const closePanel = () => setOpen(false);

// Interactions outside of the component when it is open should close it
//
// @ts-expect-error - `useElementShouldClose` requires a non-nullable ref,
// but `shareRef` is nullable.
useElementShouldClose(shareRef, isOpen, closePanel);

useEffect(() => {
Expand Down
2 changes: 1 addition & 1 deletion src/sidebar/components/Excerpt.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ function Excerpt({
useState(true);

// Container for the excerpt's content.
const contentElement = useRef(/** @type {HTMLDivElement|null} */ (null));
const contentElement = /** @type {{ current: HTMLDivElement }} */ (useRef());

// Measured height of `contentElement` in pixels.
const [contentHeight, setContentHeight] = useState(0);
Expand Down
9 changes: 5 additions & 4 deletions src/sidebar/components/MarkdownEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -402,15 +402,16 @@ export default function MarkdownEditor({

useEffect(() => {
if (!preview) {
input.current.focus();
input.current?.focus();
}
}, [preview]);

const togglePreview = () => setPreview(!preview);
const handleCommand = command => {
const inputEl = input.current;
handleToolbarCommand(command, inputEl);
onEditText({ text: inputEl.value });
if (input.current) {
handleToolbarCommand(command, input.current);
onEditText({ text: input.current.value });
}
};

const handleKeyDown = event => {
Expand Down
2 changes: 1 addition & 1 deletion src/sidebar/components/MarkdownView.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default function MarkdownView({
() => (markdown ? renderMarkdown(markdown) : ''),
[markdown]
);
const content = useRef(/** @type {HTMLDivElement|null} */ (null));
const content = /** @type {{ current: HTMLDivElement }} */ (useRef());

useEffect(() => {
replaceLinksWithEmbeds(content.current, {
Expand Down
2 changes: 1 addition & 1 deletion src/sidebar/components/Menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ export default function Menu({
//
// These handlers close the menu when the user taps or clicks outside the
// menu or presses Escape.
const menuRef = useRef(/** @type {HTMLDivElement|null} */ (null));
const menuRef = /** @type {{ current: HTMLDivElement }} */ (useRef());

// Menu element should close via `closeMenu` whenever it's open and there
// are user interactions outside of it (e.g. clicks) in the document
Expand Down
6 changes: 3 additions & 3 deletions src/sidebar/components/MenuItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ export default function MenuItem({
const hasLeftIcon = icon || isSubmenuItem;
const hasRightIcon = icon && isSubmenuItem;

const menuItemRef = useRef(
/** @type {(HTMLAnchorElement & HTMLDivElement)|null} */ (null)
);
const menuItemRef =
/** @type {{ current: HTMLAnchorElement & HTMLDivElement }} */ (useRef());

let focusTimer = null;

let renderedIcon = null;
Expand Down
2 changes: 1 addition & 1 deletion src/sidebar/components/MenuKeyboardNavigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default function MenuKeyboardNavigation({
children,
visible,
}) {
const menuRef = useRef(/** @type {HTMLDivElement|null} */ (null));
const menuRef = /** @type {{ current: HTMLDivElement }} */ (useRef());

useEffect(() => {
let focusTimer = null;
Expand Down
2 changes: 1 addition & 1 deletion src/sidebar/components/SearchInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { useStoreProxy } from '../store/use-store';
export default function SearchInput({ alwaysExpanded, query, onSearch }) {
const store = useStoreProxy();
const isLoading = store.isLoading();
const input = useRef(/** @type {HTMLInputElement|null} */ (null));
const input = /** @type {{ current: HTMLInputElement }} */ (useRef());

// The active filter query from the previous render.
const [prevQuery, setPrevQuery] = useState(query);
Expand Down
2 changes: 1 addition & 1 deletion src/sidebar/components/Slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { useCallback, useEffect, useRef, useState } from 'preact/hooks';
* @param {SliderProps} props
*/
export default function Slider({ children, visible }) {
const containerRef = useRef(/** @type {HTMLDivElement|null} */ (null));
const containerRef = /** @type {{ current: HTMLDivElement }} */ (useRef());
const [containerHeight, setContainerHeight] = useState(visible ? 'auto' : 0);

// Whether the content is currently partially or wholly visible. This is
Expand Down
4 changes: 2 additions & 2 deletions src/sidebar/components/TagEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function TagEditor({
tagList,
tags: tagsService,
}) {
const inputEl = useRef(/** @type {HTMLInputElement|null} */ (null));
const inputEl = /** @type {{ current: HTMLInputElement }} */ (useRef());
const [suggestions, setSuggestions] = useState(/** @type {string[]} */ ([]));
const [activeItem, setActiveItem] = useState(-1); // -1 is unselected
const [suggestionsListOpen, setSuggestionsListOpen] = useState(false);
Expand All @@ -48,7 +48,7 @@ function TagEditor({
});

// Set up callback to monitor outside click events to close the AutocompleteList
const closeWrapperRef = useRef(/** @type {HTMLElement|null} */ (null));
const closeWrapperRef = /** @type {{ current: HTMLElement }} */ (useRef());
useElementShouldClose(closeWrapperRef, suggestionsListOpen, () => {
setSuggestionsListOpen(false);
});
Expand Down
2 changes: 1 addition & 1 deletion src/sidebar/store/use-store.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,5 +144,5 @@ export function useStoreProxy() {
return cleanup;
}, [cache, store]);

return proxy.current;
return /** @type {SidebarStore} */ (proxy.current);
}

0 comments on commit 8ee4cec

Please sign in to comment.