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

Feedback needed: add option to enable effects and lifecycle methods in shallow renderer #15589

Closed
wants to merge 1 commit into from
Closed
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
41 changes: 33 additions & 8 deletions packages/react-test-renderer/src/ReactShallowRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,12 @@ function basicStateReducer<S>(state: S, action: BasicStateAction<S>): S {
}

class ReactShallowRenderer {
static createRenderer = function() {
return new ReactShallowRenderer();
static createRenderer = function(useLifecycleMethods) {
return new ReactShallowRenderer(useLifecycleMethods);
};

constructor() {
constructor(useLifecycleMethods = false) {
Copy link
Author

Choose a reason for hiding this comment

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

The way this option is passed can be changed. I just did this for now because it was easy.

this._useLifecycleMethods = useLifecycleMethods;
this._reset();
}

Expand Down Expand Up @@ -366,6 +367,13 @@ class ReactShallowRenderer {
return fn;
};

const useEffect = (fn: Function, deps: Array<mixed> | void | null): Function => {
this._validateCurrentlyRenderingComponent();
this._createWorkInProgressHook();

//fill in impl
Copy link
Author

Choose a reason for hiding this comment

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

This is the part I'm trying to still figure out how to do. I think I have a general idea of how things work, but before I went any further I wanted to confirm that this makes sense.

};

return {
readContext,
useCallback: (identity: any),
Expand All @@ -374,9 +382,9 @@ class ReactShallowRenderer {
return readContext(context);
},
useDebugValue: noOp,
useEffect: noOp,
useEffect: this._useLifecycleMethods ? useEffect : noOp,
useImperativeHandle: noOp,
useLayoutEffect: noOp,
useLayoutEffect: this._useLifecycleMethods ? useEffect : noOp,
useMemo,
useReducer,
useRef,
Expand Down Expand Up @@ -674,8 +682,13 @@ class ReactShallowRenderer {
}

this._rendered = this._instance.render();
// Intentionally do not call componentDidMount()
// Force user to opt in to calling componentDidMount()
// because DOM refs are not available.
if(this._useLifecycleMethods) {

Choose a reason for hiding this comment

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

spacing

Copy link
Author

Choose a reason for hiding this comment

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

I'm still trying to get feedback on the implementation plan. I'll clean up spacing and things like that at the end

if (typeof this._instance.componentDidMount === 'function') {
this._instance.componentDidMount();
}
}
}

_updateClassComponent(
Expand Down Expand Up @@ -760,9 +773,21 @@ class ReactShallowRenderer {

if (shouldUpdate) {
this._rendered = this._instance.render();

// Force user to opt in to calling componentDidUpdate() and
// getSnapshotBeforeUpdate because DOM refs are not available.
if(this._useLifecycleMethods) {

Choose a reason for hiding this comment

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

spacing

let snapshot = null;

if (typeof this._instance.getSnapshotBeforeUpdate === 'function') {
snapshot = this._instance.getSnapshotBeforeUpdate(oldProps, oldState);
}

if (typeof this._instance.componentDidUpdate === 'function') {
this._instance.componentDidUpdate(oldProps, oldState, snapshot);
}
}
}
// Intentionally do not call componentDidUpdate()
// because DOM refs are not available.
}
}

Expand Down