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 instances of this pattern to one of the
following:

```
const nonNullRef = /** @type {{ current: T }} */ (useRef());
const nullRef = /** @type {{ current: T|null }} */ (useRef(null));
```

Where the first pattern is used for refs that will always be set after
the initial render, and the second case is used for refs that may not be
set (eg. if the element is conditionally rendered).
  • Loading branch information
robertknight committed Nov 10, 2021
1 parent 63020d6 commit 2bfa29e
Show file tree
Hide file tree
Showing 13 changed files with 29 additions and 28 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 = /** @type {{ current: Emitter|null }} */ (useRef(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
2 changes: 1 addition & 1 deletion src/sidebar/components/MarkdownEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ export default function MarkdownEditor({
const [preview, setPreview] = useState(false);

// The input element where the user inputs their comment.
const input = useRef(/** @type {HTMLTextAreaElement|null} */ (null));
const input = /** @type {{current: HTMLTextAreaElement}} */ (useRef());

useEffect(() => {
if (!preview) {
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 2bfa29e

Please sign in to comment.