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

Bump preact from 10.5.13 to 10.5.15 #3845

Merged
merged 2 commits into from
Nov 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a bug here, but since it is not a new one, I just suppressed the error for the moment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shareRef is a non null ref, why not to use the pattern you suggested: const shareRef = /** @type {{ current: HTMLDivElement }} */ (useRef());

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shareRef is a nullable ref! - There is a return null statement in the component mid-way down with an issue link above it.

Copy link
Contributor

@esanzgar esanzgar Nov 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. I was looking for return null but closer to the return of the component and I didn't find it...

Then, yes I agree that adding the ts-expect-error is the best option for now.


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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is non-null cast here, which we know is safe in this context.

}
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5937,9 +5937,9 @@ postcss@^8.0.3:
source-map-js "^0.6.2"

preact@^10.4.0:
version "10.5.13"
resolved "https://registry.yarnpkg.com/preact/-/preact-10.5.13.tgz#85f6c9197ecd736ce8e3bec044d08fd1330fa019"
integrity sha512-q/vlKIGNwzTLu+jCcvywgGrt+H/1P/oIRSD6mV4ln3hmlC+Aa34C7yfPI4+5bzW8pONyVXYS7SvXosy2dKKtWQ==
version "10.5.15"
resolved "https://registry.yarnpkg.com/preact/-/preact-10.5.15.tgz#6df94d8afecf3f9e10a742fd8c362ddab464225f"
integrity sha512-5chK29n6QcJc3m1lVrKQSQ+V7K1Gb8HeQY6FViQ5AxCAEGu3DaHffWNDkC9+miZgsLvbvU9rxbV1qinGHMHzqA==

prelude-ls@^1.2.1:
version "1.2.1"
Expand Down