Bump preact from 10.5.13 to 10.5.15 #3845

dependabot · 2021-10-18T10:03:17Z

Bumps preact from 10.5.13 to 10.5.15.

Release notes

10.5.15

Fixes

Skip rendering contents on the client in compat (#3238, thanks @developit)

Nested Suspended trees may be missing _children (#3260, thanks @developit)

Manage camel-cased dominant-baseline attribute in preact/compat (#2859, thanks @nmondon)

Restrict camelCase prop renaming to non-custom elements (#3259, thanks @jramanat-oracle)

Fix bug in chrome where select values continuously rerender (#3226, thanks @JoviDeCroock)

Avoid leaking internal vnodes (#3106, thanks @developit)

Types

Fix ref typing of forwardRef (#3214, thanks @itkrt2y)

Add focusin & focusout to type definitions (#3257, thanks @boarwell)

Add onBeforeInput attribute to type definitions (#3256, thanks @boarwell)

Add missed HTML attributes in the TS typings (#3246, thanks @rschristian)

Maintenance

Update babel.config.js (#3265, thanks @RRDAWLX)

Delete redundant previousComponent in hooks (#3271, thanks @RRDAWLX)

Update test dependencies (#3255, thanks @marvinhagemeister)

10.5.14

Features

Compat: Add flushSync (#3094, thanks @zephraph)

Bug Fixes

Prevent eager child removal (#3210, thanks @JoviDeCroock)

Compat: Update Fake React Version (#3189, thanks @tim-on-github)

Fix react-spring error caused by augmented function contexts (#3165, thanks @developit)

Use Array.prototype.slice.call for children arguments (#3143, thanks @fzzle)

Improve performance of vnodeId generation (#2978, thanks @developit)

Fix: should override children if null is provided as an argument (#3091, thanks @clyfish)

Size

refactor(diff-index): reuse i to reduce size (-32b) (#3193, thanks @liuarui)

Simplify unmount logic (#3120, thanks @andrewiggins)

Typings

Fix type definitions (#3191, thanks @craftedsystems)

Fix undefined initializer case for useState type (#3185, thanks @rschristian)

Make typings deno compatible (#3079, thanks @lucacasonato)

Change typing of event.this to be never (#3147, thanks @JoviDeCroock)

Update signature of lazy to reflect behavior (#3139, thanks @JonasKruckenberg)

Improve typing of forwardRef (#3144, thanks @cmlenz)

... (truncated)

Commits

bd52611 10.5.15 (#3282)
9ae9f03 Avoid leaking internal vnodes (#3106)
51bfe15 Use Ref to accept RefCallback (#3214)
d78746c [compat] Skip rendering <noscript> contents on the client (#3238)
6d0f682 Update babel.config.js (#3265)
4aaecc1 Merge pull request #3271 from RRDAWLX/master
2d7ba34 polish: delete redundant previousComponent in hooks
98f130e Merge pull request #3260 from preactjs/bugfix-suspended-missing-children
2624d51 Merge branch 'master' into bugfix-suspended-missing-children
fe3d375 Merge pull request #2859 from nmondon/fix-dominant-baseline-attr
Additional commits viewable in compare view

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

@dependabot rebase will rebase this PR
@dependabot recreate will recreate this PR, overwriting any edits that have been made to it
@dependabot merge will merge this PR after your CI passes on it
@dependabot squash and merge will squash and merge this PR after your CI passes on it
@dependabot cancel merge will cancel a previously requested merge and block automerging
@dependabot reopen will reopen this PR if it is closed
@dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
@dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

codecov · 2021-10-27T11:54:28Z

Codecov Report

Merging #3845 (2dd33bf) into master (8e2d612) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3845   +/-   ##
=======================================
  Coverage   98.98%   98.98%           
=======================================
  Files         211      211           
  Lines        7904     7904           
  Branches     1870     1871    +1     
=======================================
  Hits         7824     7824           
  Misses         80       80

Impacted Files	Coverage Δ
src/annotator/util/emitter.js	`100.00% <ø> (ø)`
...ar/components/Annotation/AnnotationShareControl.js	`100.00% <ø> (ø)`
src/annotator/components/NotebookModal.js	`100.00% <100.00%> (ø)`
src/sidebar/components/Excerpt.js	`92.10% <100.00%> (ø)`
src/sidebar/components/MarkdownEditor.js	`91.97% <100.00%> (ø)`
src/sidebar/components/MarkdownView.js	`100.00% <100.00%> (ø)`
src/sidebar/components/Menu.js	`100.00% <100.00%> (ø)`
src/sidebar/components/MenuItem.js	`96.55% <100.00%> (ø)`
src/sidebar/components/MenuKeyboardNavigation.js	`100.00% <100.00%> (ø)`
src/sidebar/components/SearchInput.js	`95.65% <100.00%> (ø)`
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e2d612...2dd33bf. Read the comment docs.

robertknight · 2021-10-27T12:04:43Z

I have added a commit which shows a possible solution to the increased useRef strictness in this Preact update. If we like it then we'll want to use it in other projects too, so we should move it into @hypothesis/frontend-shared or another shared package.

An alternative approach would be to patch the Preact typings locally to revert useRef to the way it worked before. The idiom we used for element refs was always kind of a hack though.

Bumps [preact](https://github.com/preactjs/preact) from 10.5.13 to 10.5.15. - [Release notes](https://github.com/preactjs/preact/releases) - [Commits](preactjs/preact@10.5.13...10.5.15) --- updated-dependencies: - dependency-name: preact dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>

robertknight · 2021-10-28T09:47:33Z

src/sidebar/components/Excerpt.js

-  }, [collapsedHeight, onCollapsibleChanged, overflowThreshold]);
+  }, [
+    collapsedHeight,
+    contentElement,


Unfortunately ESLint plugin for React doesn't know that useElementRef returns an immutable value that therefore doesn't need to be listed in the dependencies array. Would be worth checking if there is a config option to avoid this.

Unfortunately it looks like there isn't a config option to declare that a custom hook returns a stable result (ie. one that does not change across renders). See facebook/react#22539.

robertknight · 2021-10-28T09:49:43Z

src/sidebar/components/MenuItem.js

@@ -141,6 +140,7 @@ export default function MenuItem({
    // The menu item is a link
    menuItem = (
      <a
+        // @ts-ignore


This is a hack to deal with the fact that this ref may be assigned to either an HTMLAnchorElement or an HTMLDivElement and the useElementRef API only supports one type. I'm sure there will be a better way to handle this.

robertknight · 2021-10-28T09:50:29Z

src/sidebar/store/use-store.js

@@ -144,5 +144,5 @@ export function useStoreProxy() {
    return cleanup;
  }, [cache, store]);

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


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

robertknight · 2021-10-28T09:51:53Z

src/shared/hooks.js

+ * @return {{ current: InstanceType<T> }}
+ */
+// eslint-disable-next-line no-unused-vars
+export function useElementRef(type) {


The type argument is currently unused and is purely a hint to TS. We could perhaps use it for an instanceof check in the current field setter in future, maybe only in development builds.

robertknight · 2021-11-03T08:04:16Z

src/annotator/components/NotebookModal.js

-        setGroupId(groupId);
-      }
-    );
+    const newEmitter = eventBus.createEmitter();


These changes fix two complaints about emitter.current being possibly null. In the subscribe call we know that's not the case. In the cleanup callback that useEffect returns however there is the possibility that emitter.current gets re-assigned before the callback is run. The change here avoids that hazard.

esanzgar · 2021-11-10T15:45:09Z

I believe it is a good thing that preact has made the refs nullable. I think that would prevent the pitfall of believing a ref has been assigned when it didn't because it was conditionally created or rendered.

Instead of the solution presented in this PR, I would suggest to evaluate all our current usage of useRef.

I would suggest this more concise version to define the types:

const inputRef = useRef(/** @type {HTMLInputElement?} */ (null)); (notice no null in the type).

If we know for sure that certain ref will be assigned, I suggest this other type (which is not much longer):

const shareRef = /** @type {{current: HTMLDivElement}} */ (useRef());

robertknight · 2021-11-10T17:03:48Z

Hi @esanzgar - I agree with your suggestion. Casting the result of useRef does seem like a better solution. Using a custom hook for such a common use case did feel a bit un-idiomatic. I do have a slight revision to suggest though which is to make the two cases more symmetric so we don't have to rethink which to use in a given situation. Also I preferred T|null rather than T? just for consistency with existing types in the codebase.

I have pushed a replacement second commit on this branch which changes all instances of:

const someRef = useRef(/** @type {T|null} */ (null));

To one of:

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

Given how common this is in the code, we could maybe shorten it down in future by introducing a global Ref<T> alias for { current: T }.

robertknight · 2021-11-10T17:05:19Z

src/sidebar/components/Annotation/AnnotationShareControl.js

@@ -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);


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

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

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

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.

Adapt to changes in useRef types in Preact v10.5.15 by using the pattern described in hypothesis/client#3845 (comment) to cast the result of `useRef`.

robertknight · 2021-11-11T11:43:40Z

I applied the pattern mention in #3845 (comment) in the LMS app: hypothesis/lms#3234.

Adapt to changes in useRef types in Preact v10.5.15 by using the pattern described in hypothesis/client#3845 (comment) to cast the result of `useRef`.

esanzgar

I believe that the input ref in the MarkdownEditor is conditionally rendered.

You have suggested two patterns for the useRef's type:

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

However, the pattern: const nullRef = useRef(/** @type {T|null} */ (null)); is still present. Do you have plans to change that for the new nullRef style you suggest above?

esanzgar · 2021-11-11T15:00:22Z

src/sidebar/components/Annotation/AnnotationShareControl.js

@@ -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);


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

esanzgar · 2021-11-11T15:04:08Z

src/sidebar/components/MarkdownEditor.js

@@ -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());


I believe this ref could be null (it is conditionally rendered).

Oh indeed, you're right. I'll correct this.

esanzgar · 2021-11-11T15:12:36Z

src/annotator/components/NotebookModal.js

-  const emitter = useRef(
-    /** @type {ReturnType<eventBus['createEmitter']>|null} */ (null)
-  );
+  const emitterRef = /** @type {{ current: Emitter|null }} */ (useRef(null));


Maybe trimming start and end whitespaces?

Suggested change

const emitterRef = /** @type {{ current: Emitter|null }} */ (useRef(null));

const emitterRef = /** @type {{current: Emitter|null}} */ (useRef(null));

I don't have a hugely strong aesthetic preference here, but I used whitespace because that's how Prettier formats a type in TS (ie. if you write type Ref = {current: HTMLElement} in a TS file and format it) and also how the majority of our current inline object types are formatted.

@type

`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()); ```

robertknight · 2021-11-11T16:13:23Z

However, the pattern: const nullRef = useRef(/** @type {T|null} */ (null)); is still present. Do you have plans to change that for the new nullRef style you suggest above?

Hmm. Looking at this again today made me change my mind. It would have made the two styles of element ref consistent, but it would then be inconsistent with other non-element refs. I've changed it back to be much more like your original suggestion, with the only change being to use an explicit |null instead of ?, because that's how we already write nullable types in most places.

esanzgar · 2021-11-11T16:16:48Z

src/sidebar/components/Annotation/AnnotationShareControl.js

@@ -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);


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.

dependabot bot added dependencies Pull requests that update a dependency file javascript labels Oct 18, 2021

dependabot bot mentioned this pull request Oct 18, 2021

Bump preact from 10.5.13 to 10.5.14 #3557

Closed

dependabot bot assigned robertknight Oct 18, 2021

dependabot bot force-pushed the dependabot/npm_and_yarn/preact-10.5.15 branch from c8509b1 to b524ce9 Compare October 25, 2021 11:29

robertknight force-pushed the dependabot/npm_and_yarn/preact-10.5.15 branch from 2382576 to a9d6dd5 Compare October 27, 2021 12:05

robertknight mentioned this pull request Oct 27, 2021

Bump preact from 10.5.13 to 10.5.15 hypothesis/lms#3234

Merged

robertknight reviewed Oct 28, 2021

View reviewed changes

robertknight reviewed Nov 3, 2021

View reviewed changes

esanzgar self-requested a review November 10, 2021 14:29

robertknight force-pushed the dependabot/npm_and_yarn/preact-10.5.15 branch 2 times, most recently from 5797efb to 2bfa29e Compare November 10, 2021 16:57

robertknight reviewed Nov 10, 2021

View reviewed changes

robertknight force-pushed the dependabot/npm_and_yarn/preact-10.5.15 branch from 2bfa29e to 89201db Compare November 10, 2021 17:06

robertknight added a commit to hypothesis/lms that referenced this pull request Nov 11, 2021

Adapt to change in useRef types

aad91d8

Adapt to changes in useRef types in Preact v10.5.15 by using the pattern described in hypothesis/client#3845 (comment) to cast the result of `useRef`.

robertknight added a commit to hypothesis/lms that referenced this pull request Nov 11, 2021

Adapt to change in useRef types

09c5d5a

Adapt to changes in useRef types in Preact v10.5.15 by using the pattern described in hypothesis/client#3845 (comment) to cast the result of `useRef`.

esanzgar suggested changes Nov 11, 2021

View reviewed changes

robertknight force-pushed the dependabot/npm_and_yarn/preact-10.5.15 branch from 11fd7d1 to 2dd33bf Compare November 11, 2021 16:12

esanzgar approved these changes Nov 11, 2021

View reviewed changes

robertknight merged commit 8ee4cec into master Nov 11, 2021

robertknight deleted the dependabot/npm_and_yarn/preact-10.5.15 branch November 11, 2021 16:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump preact from 10.5.13 to 10.5.15 #3845

Bump preact from 10.5.13 to 10.5.15 #3845

dependabot bot commented on behalf of github Oct 18, 2021 •

edited

Loading

codecov bot commented Oct 27, 2021 •

edited

Loading

robertknight commented Oct 27, 2021 •

edited

Loading

robertknight Oct 28, 2021

robertknight Nov 3, 2021

robertknight Oct 28, 2021

robertknight Oct 28, 2021

robertknight Oct 28, 2021 •

edited

Loading

robertknight Nov 3, 2021

esanzgar commented Nov 10, 2021

robertknight commented Nov 10, 2021

robertknight Nov 10, 2021

esanzgar Nov 11, 2021

robertknight Nov 11, 2021

esanzgar Nov 11, 2021 •

edited

Loading

robertknight commented Nov 11, 2021

esanzgar left a comment

esanzgar Nov 11, 2021

esanzgar Nov 11, 2021

robertknight Nov 11, 2021

esanzgar Nov 11, 2021

robertknight Nov 11, 2021 •

edited

Loading

esanzgar Nov 11, 2021

robertknight commented Nov 11, 2021

esanzgar Nov 11, 2021 •

edited

Loading

	const emitterRef = /** @type {{ current: Emitter\|null }} */ (useRef(null));
	const emitterRef = /** @type {{current: Emitter\|null}} */ (useRef(null));

Bump preact from 10.5.13 to 10.5.15 #3845

Bump preact from 10.5.13 to 10.5.15 #3845

Conversation

dependabot bot commented on behalf of github Oct 18, 2021 • edited Loading

10.5.15

Fixes

Types

Maintenance

10.5.14

Features

Bug Fixes

Size

Typings

codecov bot commented Oct 27, 2021 • edited Loading

Codecov Report

robertknight commented Oct 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robertknight Oct 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

esanzgar commented Nov 10, 2021

robertknight commented Nov 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

esanzgar Nov 11, 2021 • edited Loading

Choose a reason for hiding this comment

robertknight commented Nov 11, 2021

esanzgar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robertknight Nov 11, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robertknight commented Nov 11, 2021

esanzgar Nov 11, 2021 • edited Loading

Choose a reason for hiding this comment

dependabot bot commented on behalf of github Oct 18, 2021 •

edited

Loading

codecov bot commented Oct 27, 2021 •

edited

Loading

robertknight commented Oct 27, 2021 •

edited

Loading

robertknight Oct 28, 2021 •

edited

Loading

esanzgar Nov 11, 2021 •

edited

Loading

robertknight Nov 11, 2021 •

edited

Loading

esanzgar Nov 11, 2021 •

edited

Loading