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

Can we locate the line number when react makes a warning like this "Unknown DOM property class. Did you mean className?" #6062

Closed
liuyutao opened this issue Feb 18, 2016 · 11 comments

Comments

@liuyutao
Copy link

Can we locate the line number when react makes a warning like this "Unknown DOM property class. Did you mean className?"

There is no hint about which code made this warning.

@jimfb
Copy link
Contributor

jimfb commented Feb 18, 2016

Yes, it is on my todo list.

@mxstbr
Copy link
Contributor

mxstbr commented Feb 18, 2016

I'd like to take a crack at this @jimfb. What do you imagine the warning is like, full filename/line number in the warning, component name,...?

@jimfb
Copy link
Contributor

jimfb commented Feb 18, 2016

@mxstbr Honestly, this probably isn't the greatest first bug, until I first blaze the trail by providing an example that you can replicate. The desired output would be something like MyFileName.js:54

You're welcomed to take a stab at it, but just know that you're probably in for a rough time in terms of a code review. To get an idea of the structure, you will need to look at #5590. You will want to emit an event when you start processing the element**, an event when you do an operation (I think that event already exists), and an event when you finish processing the element. Then your devtool can grab the __source off the element (after enabling react-jsx-source) and use that to display the file/line information as part of the error.

** At least, that would be my first approach. An alternate approach is to process the element by replicating the control logic that detects invalid props, but that feels sucky because it relies on two totally separate code paths having the same behavior and never getting out of sync. I'm not sure which approach would ultimately pan out as preferred.

@mxstbr
Copy link
Contributor

mxstbr commented Feb 18, 2016

You will want to emit an event when you start processing the element, an event when you do an operation (I think that event already exists), and an event when you finish processing the element.

Why do I need three separate events, I thought about using onSetValueForProperty in ReactDOMUnknownPropertyDevtool, and with react-jsx-source add the information to the warning – what am I missing here?

@jimfb
Copy link
Contributor

jimfb commented Feb 18, 2016

@mxstbr onSetValueForProperty doesn't have access to the element (so it can't access the source), and passing it the element (or source) would require routing the data through the core internal APIs for the sole purpose of warnings and doesn't make sense from a DOMPropertyOperations API perspective (avoiding that was one of the motivations for moving the warnings into the devtools - we want to cleanup the core internals, which currently pass data allover the place).

@mxstbr
Copy link
Contributor

mxstbr commented Feb 18, 2016

Fair enough, makes sense. I'll leave this to you then – if you have anything to do that might be a better first bug, ping me!

@jimfb
Copy link
Contributor

jimfb commented Feb 18, 2016

@mxstbr We have a whole list of recommended ones here: https://github.com/facebook/react/labels/good%20first%20bug

In particular, these jumped out at me: #5839 #6049 #1858

@mxstbr
Copy link
Contributor

mxstbr commented Feb 18, 2016

#1858 was done in #2065 and can be closed, #5839 has an open PR at #5861 and tleunen reserved #6049. I'll keep a lookout at the list though!

troydemonbreun added a commit to troydemonbreun/react that referenced this issue Apr 1, 2016
troydemonbreun added a commit to troydemonbreun/react that referenced this issue Apr 1, 2016
troydemonbreun added a commit to troydemonbreun/react that referenced this issue Apr 1, 2016
troydemonbreun added a commit to troydemonbreun/react that referenced this issue Apr 4, 2016
troydemonbreun added a commit to troydemonbreun/react that referenced this issue Apr 5, 2016
troydemonbreun added a commit to troydemonbreun/react that referenced this issue Apr 5, 2016
jimfb pushed a commit that referenced this issue May 3, 2016
…6398)

* New approach for 6062 fix : Show source line number on unknown property warning

* WIP: ReactDebugToolEventForwarderDevTool

* Update event signature to debugID

* Trigger events in ReactDOMComponent

* Renamed to onMountDOMComponent; passing in element directly

* Added debugID; updated simple test

* Added test for multi-div JSX to ref exact line

* Added test for composite component
zpao pushed a commit to zpao/react that referenced this issue Jun 8, 2016
…warning (facebook#6398)

* New approach for 6062 fix : Show source line number on unknown property warning

* WIP: ReactDebugToolEventForwarderDevTool

* Update event signature to debugID

* Trigger events in ReactDOMComponent

* Renamed to onMountDOMComponent; passing in element directly

* Added debugID; updated simple test

* Added test for multi-div JSX to ref exact line

* Added test for composite component

(cherry picked from commit 7cf61db)
zpao pushed a commit that referenced this issue Jun 14, 2016
…6398)

* New approach for 6062 fix : Show source line number on unknown property warning

* WIP: ReactDebugToolEventForwarderDevTool

* Update event signature to debugID

* Trigger events in ReactDOMComponent

* Renamed to onMountDOMComponent; passing in element directly

* Added debugID; updated simple test

* Added test for multi-div JSX to ref exact line

* Added test for composite component

(cherry picked from commit 7cf61db)
@gaearon
Copy link
Collaborator

gaearon commented Oct 27, 2016

We do have the line numbers for this now if you enable this transform in development. It's already included by default with Create React App.

@Domiii
Copy link

Domiii commented May 1, 2018

Just tried out babel-plugin-transform-react-jsx-source, and it works! Thanks so much @gaearon!

"How to display line numbers in React render errors?" - The answer to that question should be in every React beginner tutorial, if not shown in the error message itself!

@gaearon
Copy link
Collaborator

gaearon commented May 1, 2018

Well, we do recommend using Create React App in beginner tutorials, including those we have on our own website.

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

No branches or pull requests

5 participants