Skip to content

Yellow Box for react-dom#7360

Closed
keyz wants to merge 8 commits into
react:masterfrom
keyz:yellow-box
Closed

Yellow Box for react-dom#7360
keyz wants to merge 8 commits into
react:masterfrom
keyz:yellow-box

Conversation

@keyz

@keyz keyz commented Jul 27, 2016

Copy link
Copy Markdown
Contributor

This is a port of React Native's Yellow Box to react-dom. It's also similar to our internal ErrorMessageConsole module at Facebook.

It requires facebook/fbjs#165 which passes warning info from warning(...) to __showWarning(...) (the new global hook installed by react-dom).

A demo can be found here: https://jsfiddle.net/keyanzhang/mmupy3zh/15/embedded/result/. There's no fancy styling since we want to keep this component as simple as possible.

Thanks for reviewing! @spicyj


const YellowBoxMessage = require('ReactDOMYellowBoxMessage');

import type {Format, Instance, Milliseconds, InstanceInfo} from 'reactShowWarningDOM';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This depends on a later commit. :\

@keyz keyz Aug 2, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oops, sorry didn't notice that :|

@keyz

keyz commented Sep 27, 2016

Copy link
Copy Markdown
Contributor Author

Just rebased and updated. https://jsfiddle.net/keyanzhang/mmupy3zh/15/embedded/result/

@addyosmani

Copy link
Copy Markdown
Contributor

Beyond another rebase, is there further work to be done here before this lands? :)

@aweary

aweary commented Mar 6, 2017

Copy link
Copy Markdown
Contributor

@addyosmani it depends on facebook/fbjs#165 (which itself requires some rebasing), and it will also need to be implemented in ReactDOMFiber.

It's not clear what the status is here since it's been sitting here for so long without review, maybe @spicyj or @gaearon can clarify whether this is something that we still want to do.

@acdlite

acdlite commented Mar 6, 2017

Copy link
Copy Markdown
Collaborator

I believe this is on pause until we figure out a good public API for exposing hooks to warning and error reporting. @bvaughn built a version we use internally at Facebook that will inform what we do in open source.

@bvaughn

bvaughn commented Mar 6, 2017

Copy link
Copy Markdown
Contributor

There was also a related exploration on #8861 but as Andrew said, the team decided to put yellow box on hold for now in favor of higher-priority v16 stuff. We'll revisit it soon hopefully. 😄

@aweary

aweary commented Mar 6, 2017

Copy link
Copy Markdown
Contributor

Thanks for clarifying @acdlite @bvaughn! Should we close this then if it's not currently slated to be merged and isn't likely the implementation that will be used?

@bvaughn

bvaughn commented Mar 6, 2017

Copy link
Copy Markdown
Contributor

I think it's okay to close this PR for now. We may still end up using this as a starting point for whatever we do merge, but it's unlikely to be merged as-is.

@aweary aweary closed this Mar 6, 2017
@aweary

aweary commented Mar 6, 2017

Copy link
Copy Markdown
Contributor

@addyosmani thanks for bringing this to our attention again!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants