The Wayback Machine - https://web.archive.org/web/20200225150548/https://github.com/facebook/react/issues/14442
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

Open
mpmaia opened this issue Dec 14, 2018 · 14 comments
Open

ShallowRenderer does not work with Class.contextType #14442

mpmaia opened this issue Dec 14, 2018 · 14 comments

Comments

@mpmaia
Copy link

@mpmaia 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

This comment has been minimized.

Copy link

@charBap charBap commented Jan 19, 2019

Can I take this?

@gaearon

This comment has been minimized.

Copy link
Member

@gaearon gaearon commented Jan 19, 2019

Sure.

@Timbo2000aa

This comment has been minimized.

Copy link

@Timbo2000aa Timbo2000aa commented Feb 3, 2019

@Aberman12

This comment has been minimized.

Copy link

@Aberman12 Aberman12 commented Feb 4, 2019

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

@gaearon

This comment has been minimized.

Copy link
Member

@gaearon gaearon commented Feb 4, 2019

@Aberman12 please do!

@Aberman12

This comment has been minimized.

Copy link

@Aberman12 Aberman12 commented Feb 4, 2019

awesome, thanks!

@AbhishekThorat

This comment has been minimized.

Copy link

@AbhishekThorat 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

This comment has been minimized.

Copy link

@manuelbertelli manuelbertelli commented May 2, 2019

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

This comment has been minimized.

Copy link
Contributor

@bedakb 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

This comment has been minimized.

Copy link

@xmd5a 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

This comment has been minimized.

Copy link
Contributor

@bedakb 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

This comment has been minimized.

Copy link

@najeeb-rifaat 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 ?

@alanchavez88

This comment has been minimized.

Copy link

@alanchavez88 alanchavez88 commented Jun 25, 2019

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

@16bitash

This comment has been minimized.

Copy link

@16bitash 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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.