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

Warning in IE 11 when using React.Fragment #12500

Closed
heikkilamarko opened this issue Mar 31, 2018 · 14 comments
Closed

Warning in IE 11 when using React.Fragment #12500

heikkilamarko opened this issue Mar 31, 2018 · 14 comments

Comments

@heikkilamarko
Copy link
Contributor

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

bug

What is the current behavior?

With React 16.3.0, when using <React.Fragment> IE 11 gives the following warning:

Warning: Invalid prop `children` supplied to `React.Fragment`. React.Fragment can only have `key` and `children` props.

Steps to reproduce:

  1. Use create-react-app to create a new React app.
  2. Add <React.Fragment> to App.js. For example:
import React, { Component } from "react";
import logo from "./logo.svg";
import "./App.css";

class App extends Component {
  render() {
    return (
      <div className="App">
        <React.Fragment>
          <header className="App-header">
            <img src={logo} className="App-logo" alt="logo" />
            <h1 className="App-title">Welcome to React</h1>
          </header>
          <p className="App-intro">
            To get started, edit <code>src/App.js</code> and save to reload.
          </p>
        </React.Fragment>
      </div>
    );
  }
}

export default App;
  1. Open the app in IE 11.
  2. Open IE dev tools and refresh the browser.
  3. You should see the above mentioned warning message in the console window.

No warnings with Chrome, Firefox, and Edge.

What is the expected behavior?

There should be no warnings shown.

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

React: 16.3.0
Browser: IE 11
OS: Windows 10

@gaearon
Copy link
Collaborator

gaearon commented Mar 31, 2018

Want to look into why this happens?

@heikkilamarko
Copy link
Contributor Author

In ReactElementValidator.js

VALID_FRAGMENT_PROPS = new Map([['children', true], ['key', true]]);

With IE 11, after the above statement, the size of VALID_FRAGMENT_PROPS is zero.

Because of this, the following function outputs the warning:

function validateFragmentProps(fragment) {
  currentlyValidatingElement = fragment;

  const keys = Object.keys(fragment.props);
  for (let i = 0; i < keys.length; i++) {
    const key = keys[i];
    if (!VALID_FRAGMENT_PROPS.has(key)) {
      warning(
        false,
        'Invalid prop `%s` supplied to `React.Fragment`. ' +
          'React.Fragment can only have `key` and `children` props.%s',
        key,
        getStackAddendum(),
      );
      break;
    }
  }

@jquense
Copy link
Contributor

jquense commented Mar 31, 2018

How are you polyfilling Map? This seems like maybe the same issue as [on a phone can't find the other issue], where the problem was ie11's Map is broken for ctor arguments like this

@gaearon
Copy link
Collaborator

gaearon commented Mar 31, 2018

Ugh right. Not our first time bumping into this, maybe it’s time to add an internal rule against Map/Set constructor argument.

Do you want to send a fix?

@gaearon
Copy link
Collaborator

gaearon commented Mar 31, 2018

The thing is, we don’t require Map to be polyfilled on IE11 because it already exists there. We just need to avoid constructor arguments in our code.

@jquense
Copy link
Contributor

jquense commented Mar 31, 2018

Could also do a quick test when check for broken Map implementations. Avoiding the ctor argument is easy enough for React, too. It might nicer for the ecosystem to insist on a valid Map, since libraries will be using Map/Set more assuming the React env baseline.

@heikkilamarko
Copy link
Contributor Author

Browser compatibility table shows that new Map(iterable) is not supported in IE 11.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map

I tried the following code and it makes the warning go away.

VALID_FRAGMENT_PROPS = new Map();
VALID_FRAGMENT_PROPS.set('children', true);
VALID_FRAGMENT_PROPS.set('key', true);

@gaearon
Copy link
Collaborator

gaearon commented Mar 31, 2018

Let’s just replace this with two string literal comparisons. I don’t see why the Map here is useful.

@heikkilamarko
Copy link
Contributor Author

#12504

@aweary
Copy link
Contributor

aweary commented Mar 31, 2018

This brings up a broader question on how to handle bad native implementations. These cases are easy enough to refactor, but what happens when we really want to use a Map/Set/etc. feature that isn't available in some native implementations? Does it make sense to ask users to polyfill for bad native behavior too?

@gaearon
Copy link
Collaborator

gaearon commented Apr 1, 2018

We decide this on a case by case basis. In case of Map/Set we're happy with IE11 feature set (afaik), we just keep forgetting about it.

@gaearon
Copy link
Collaborator

gaearon commented Apr 4, 2018

Fixed in 16.3.1

@ImanMh
Copy link

ImanMh commented Jun 5, 2018

@gaearon I use React ^16.3.2 an I yet have this problem and it's not only IE but FF too. Is it possible that the issue still exists? How can I fix it and send a pr?

@ImanMh
Copy link

ImanMh commented Jun 5, 2018

I passed fragment to a library and that library is trying to add class names to the fragment children. I think that's why It happens to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants