Skip to content

Commit

Permalink
Backport changes from 10.6.0-10.6.2 (#3357)
Browse files Browse the repository at this point in the history
* fix className leak

* export ComponentProps from compat

* fix useRef types

* adjsut raf types

* fix erroring hooks

* fix json files

* add depth ordering

* alias onfocusin

* bump check-export-map
  • Loading branch information
JoviDeCroock committed Dec 7, 2021
1 parent 107df6b commit 424e426
Show file tree
Hide file tree
Showing 9 changed files with 157 additions and 47 deletions.
1 change: 1 addition & 0 deletions compat/src/index.d.ts
Expand Up @@ -39,6 +39,7 @@ declare namespace React {
export import Fragment = preact.Fragment;
export import createElement = preact.createElement;
export import cloneElement = preact.cloneElement;
export import ComponentProps = preact.ComponentProps;

// Suspense
export import Suspense = _Suspense.Suspense;
Expand Down
14 changes: 9 additions & 5 deletions compat/src/render.js
Expand Up @@ -162,6 +162,10 @@ options.vnode = vnode => {
!onChangeInputType(props.type)
) {
i = 'oninput';
} else if (/^onfocus$/i.test(i)) {
i = 'onfocusin';
} else if (/^onblur$/i.test(i)) {
i = 'onfocusout';
} else if (/^on(Ani|Tra|Tou|BeforeInp)/.test(i)) {
i = i.toLowerCase();
} else if (nonCustomElement && CAMEL_PROPS.test(i)) {
Expand Down Expand Up @@ -200,12 +204,12 @@ options.vnode = vnode => {
}

vnode.props = normalizedProps;
}

if (type && props.class != props.className) {
classNameDescriptor.enumerable = 'className' in props;
if (props.className != null) normalizedProps.class = props.className;
Object.defineProperty(normalizedProps, 'className', classNameDescriptor);
if (type && props.class != props.className) {
classNameDescriptor.enumerable = 'className' in props;
if (props.className != null) normalizedProps.class = props.className;
Object.defineProperty(normalizedProps, 'className', classNameDescriptor);
}
}

if (typeof type === 'function' && props.ref) {
Expand Down
15 changes: 15 additions & 0 deletions compat/test/browser/render.test.js
Expand Up @@ -258,6 +258,21 @@ describe('compat render', () => {
expect(scratch.firstChild.className).to.equal('new');
});

it('should correctly allow for "className"', () => {
const Foo = props => {
const { className, ...rest } = props;
return (
<div class={className}>
<p {...rest}>Foo</p>
</div>
);
};

render(<Foo className="foo" />, scratch);
expect(scratch.firstChild.className).to.equal('foo');
expect(scratch.firstChild.firstChild.className).to.equal('');
});

it('should give precedence to last-applied class/className prop', () => {
render(<ul className="from className" class="from class" />, scratch);
expect(scratch.firstChild.className).to.equal('from className');
Expand Down
18 changes: 12 additions & 6 deletions hooks/src/index.d.ts
@@ -1,4 +1,4 @@
import { PreactContext, Ref as PreactRef, RefObject } from '../..';
import { PreactContext, Ref as PreactRef } from '../..';

type Inputs = ReadonlyArray<unknown>;

Expand Down Expand Up @@ -46,8 +46,14 @@ export function useReducer<S, A, I>(
): [S, (action: A) => void];

/** @deprecated Use the `Ref` type instead. */
type PropRef<T> = { current: T };
type Ref<T> = { current: T };
type PropRef<T> = MutableRef<T>;
interface Ref<T> {
readonly current: T | null;
}

interface MutableRef<T> {
current: T;
}

/**
* `useRef` returns a mutable ref object whose `.current` property is initialized to the passed argument
Expand All @@ -58,9 +64,9 @@ type Ref<T> = { current: T };
*
* @param initialValue the initial value to store in the ref object
*/
export function useRef<T>(initialValue: null): RefObject<T>;
export function useRef<T>(initialValue: T): Ref<T>;
export function useRef<T>(): Ref<T | undefined>;
export function useRef<T>(initialValue: T): MutableRef<T>;
export function useRef<T>(initialValue: T | null): Ref<T>;
export function useRef<T = undefined>(): MutableRef<T | undefined>;

type EffectCallback = () => void | (() => void);
/**
Expand Down
29 changes: 19 additions & 10 deletions hooks/src/index.js
@@ -1,5 +1,5 @@
import { options } from 'preact';
import { getParentContext } from 'preact/src/tree';
import { getParentContext } from '../../src/tree';
import { MODE_UNMOUNTING } from '../../src/constants';

/** @type {number} */
Expand Down Expand Up @@ -76,11 +76,15 @@ options.unmount = internal => {
if (oldBeforeUnmount) oldBeforeUnmount(internal);

if (internal.data && internal.data.__hooks) {
try {
internal.data.__hooks._list.forEach(invokeCleanup);
} catch (e) {
options._catchError(e, internal);
}
let hasErrored;
internal.data.__hooks._list.forEach(s => {
try {
invokeCleanup(s);
} catch (e) {
hasErrored = e;
}
});
if (hasErrored) options._catchError(hasErrored, internal);
}
};

Expand Down Expand Up @@ -307,7 +311,9 @@ export function useErrorBoundary(cb) {
* After paint effects consumer.
*/
function flushAfterPaintEffects() {
afterPaintEffects.forEach(internal => {
let internal;
afterPaintEffects.sort((a, b) => a._depth - b._depth);
while ((internal = afterPaintEffects.pop())) {
if (~internal.flags & MODE_UNMOUNTING) {
try {
internal.data.__hooks._pendingEffects.forEach(invokeCleanup);
Expand All @@ -318,8 +324,7 @@ function flushAfterPaintEffects() {
options._catchError(e, internal);
}
}
});
afterPaintEffects = [];
}
}

let HAS_RAF = typeof requestAnimationFrame == 'function';
Expand Down Expand Up @@ -369,7 +374,11 @@ function invokeCleanup(hook) {
// A hook cleanup can introduce a call to render which creates a new root, this will call options.vnode
// and move the currentInternal away.
const internal = currentInternal;
if (typeof hook._cleanup == 'function') hook._cleanup();
let cleanup = hook._cleanup;
if (typeof cleanup == 'function') {
hook._cleanup = undefined;
cleanup();
}
currentInternal = internal;
}

Expand Down
70 changes: 70 additions & 0 deletions hooks/test/browser/useEffect.test.js
Expand Up @@ -372,4 +372,74 @@ describe('useEffect', () => {
'<div><div>Count: 2</div><div><div>dummy</div></div></div>'
);
});

it('handles errors correctly', () => {
class ErrorBoundary extends Component {
constructor(props) {
super(props);
this.state = { error: null };
}

componentDidCatch(error) {
this.setState({ error: 'oh no' });
}

render() {
return this.state.error ? (
<h2>Error! {this.state.error}</h2>
) : (
this.props.children
);
}
}

let update;
const firstEffectSpy = sinon.spy();
const firstEffectcleanup = sinon.spy();
const secondEffectSpy = sinon.spy();
const secondEffectcleanup = sinon.spy();

const MainContent = () => {
const [val, setVal] = useState(false);

update = () => setVal(!val);
useEffect(() => {
firstEffectSpy();
return () => {
firstEffectcleanup();
throw new Error('oops');
};
}, [val]);

useEffect(() => {
secondEffectSpy();
return () => {
secondEffectcleanup();
};
}, []);

return <h1>Hello world</h1>;
};

act(() => {
render(
<ErrorBoundary>
<MainContent />
</ErrorBoundary>,
scratch
);
});

expect(firstEffectSpy).to.be.calledOnce;
expect(secondEffectSpy).to.be.calledOnce;

act(() => {
update();
});

expect(firstEffectSpy).to.be.calledOnce;
expect(secondEffectSpy).to.be.calledOnce;
expect(firstEffectcleanup).to.be.calledOnce;
expect(secondEffectcleanup).to.be.calledOnce;
});
});
46 changes: 23 additions & 23 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions package.json
Expand Up @@ -76,7 +76,12 @@
"require": "./compat/server.js"
},
"./package.json": "./package.json",
"./": "./"
"./compat/package.json": "./compat/package.json",
"./debug/package.json": "./debug/package.json",
"./devtools/package.json": "./devtools/package.json",
"./hooks/package.json": "./hooks/package.json",
"./test-utils/package.json": "./test-utils/package.json",
"./jsx-runtime/package.json": "./jsx-runtime/package.json"
},
"license": "MIT",
"funding": {
Expand Down Expand Up @@ -240,7 +245,7 @@
"babel-plugin-transform-rename-properties": "0.1.0",
"benchmark": "^2.1.4",
"chai": "^4.1.2",
"check-export-map": "^1.0.1",
"check-export-map": "^1.2.0",
"coveralls": "^3.0.0",
"cross-env": "^7.0.2",
"csstype": "^3.0.5",
Expand Down
2 changes: 1 addition & 1 deletion src/index.d.ts
Expand Up @@ -265,7 +265,7 @@ export interface Options {
/** Attach a hook that is invoked after a vnode has rendered. */
diffed?(vnode: VNode): void;
event?(e: Event): any;
requestAnimationFrame?: typeof requestAnimationFrame;
requestAnimationFrame?(callback: () => void): void;
debounceRendering?(cb: () => void): void;
useDebugValue?(value: string | number): void;
_addHookName?(name: string | number): void;
Expand Down

0 comments on commit 424e426

Please sign in to comment.