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

ShallowRenderer does not work with Class.contextType #14442

Closed
mpmaia opened this issue Dec 14, 2018 · 15 comments
Closed

ShallowRenderer does not work with Class.contextType #14442

mpmaia opened this issue Dec 14, 2018 · 15 comments

Comments

@mpmaia
Copy link

mpmaia commented Dec 14, 2018

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

The shallow renderer from the 'react-test-renderer' package does not work with Class.contextType. The component always receives an empty object as the context.

What is the expected behavior?

The shallow renderer should forward the context provided to the render method to the rendered component's this.context.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

16.6.3

Steps to reproduce

The test below can be used to reproduce the problem (Open on codesandbox.io). The first test reproduces the bug and the second test shows a workaround.

import * as React from "react";
import ShallowRenderer from 'react-test-renderer/shallow'
import PropTypes from 'prop-types';

const TestContext = React.createContext({message:"TEST1"});
class TestShallow extends React.Component {

    static contextType = TestContext;

    componentDidMount() {
        console.log(this.context.message);
    }

    render() {
        return <h1>{this.context.message}</h1>;
    }
}

it('shallow render with contextType', () => {

    const result = new ShallowRenderer().render(<TestShallow />, {message:"TEST2"});
    expect(result).toEqual(<h1>TEST2</h1>); //FAILS

});

it('workaround shallow render with contextType', () => {

    TestShallow.contextTypes = {
        message: PropTypes.string
    };

    const result = new ShallowRenderer().render(<TestShallow />, {message:"TEST2"});
    expect(result).toEqual(<h1>TEST2</h1>); //WORKS

});

The problem seems to be located here. The function getMaskedContext() checks the properties declared on type.contextTypes and filters out the properties from the context provided to the shallow render unless the types.contextTypes is declared.

@charBap
Copy link

charBap commented Jan 19, 2019

Can I take this?

@gaearon
Copy link
Collaborator

gaearon commented Jan 19, 2019

Sure.

@Timbo2000aa
Copy link

Timbo2000aa commented Feb 3, 2019 via email

@Aberman12
Copy link

Is this issue still being worked on @charBap? If not id be glad to take it on! @gaearon

@gaearon
Copy link
Collaborator

gaearon commented Feb 4, 2019

@Aberman12 please do!

@Aberman12
Copy link

awesome, thanks!

@AbhishekThorat
Copy link

AbhishekThorat commented Feb 24, 2019

Is this being worked on @Aberman12? If not I would love to look into it :)
Meanwhile, it seems that solution for this will revert changes which @koba04 has added as part of #11862 (As per my understanding, these changes are not needed with a combination of the new context API)
@gaearon and team please correct if my understanding is wrong. (I am quite new to open source)

@manuelbertelli
Copy link

Hi, I want to start collaborating with open source often and, as I had worked with React for some time, I thought it would be nice to start collaborating here. I've been looking into this issue and I suppose it's a user mistake and not a bug.

First, there is a typo in static contextType = TestContext, as it should be declared contextTypes in plural.

Second, I dig into the ReactContext to understand it more, and I suppose that it isn't used for creating element contexts, but has some use in server-side rendering and concurrency with threads.

So, trying to use React.createContext() to create element contexts never gonna work.

Saying this, IMO, this is not a bug and can be closed.

@bedakb
Copy link
Contributor

bedakb commented May 18, 2019

It's not a typo, It's more like leftover from Legacy Context API where It was in plural.

If I am not mistaken, changing contextTypes to contextType in this file https://github.com/facebook/react/blob/master/packages/react-test-renderer/src/ReactShallowRenderer.js should fix problem ?

@gaearon I'd like then take this one, If It's possible.

@xmd5a
Copy link

xmd5a commented May 24, 2019

I'm challenging this issue too.

I'm trying to get snapshots tests - changing static contextType into static contextTypes resolves my snap well, but on the other side (app) I'm getting errors. Probably changing this property name you listed in package above will fix problem.

@bedakb
Copy link
Contributor

bedakb commented May 24, 2019

I'm a bit confused with this one now.I went through the React source code and seen a lot of usages of contextTypes, so I'm not really sure is renaming thing way to go.
I'd like to have feedback from someone who is more familiar about this topic than me :)

@najeeb-rifaat
Copy link

najeeb-rifaat commented May 30, 2019

Seems like react-test-renderer still expects the old class component propretry contextTypes (plural with s)

this._context = getMaskedContext(element.type.contextTypes, context); here.

while newer (unsable) code is using class component propretry 'contextType' and some existing code suggests that both should be used while newer convention should take precedence over the older one. here

@gaearon should the fix be considering both old and new component propreties OR omit the old plural contextTypes with a warning ?

@bitforth
Copy link

Is this issue taken? I'd love to work on it.

@16bitash
Copy link

16bitash commented Jul 1, 2019

Hi, just wanted to check if someone is working on this issue. If not, it'll be my pleasure to work on it!

@gaearon
Copy link
Collaborator

gaearon commented Mar 18, 2020

The shallow renderer has moved to https://github.com/NMinhNguyen/react-shallow-renderer. If this is still relevant, please feel free to file this issue there. Thanks!

@gaearon gaearon closed this as completed Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.