Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -391,16 +391,19 @@ describe('DOMPropertyOperations', function() {
describe('injectDOMPropertyConfig', function() {
it('should support custom attributes', function() {
// foobar does not exist yet
spyOn(console, 'error');
expect(DOMPropertyOperations.createMarkupForProperty(
'foobar',
'simple'
)).toBe(null);
expect(console.error.argsForCall.length).toBe(1);

// foo-* does not exist yet
expect(DOMPropertyOperations.createMarkupForProperty(
'foo-xyz',
'simple'
)).toBe(null);
expect(console.error.argsForCall.length).toBe(2);

// inject foobar DOM property
DOMProperty.injection.injectDOMPropertyConfig({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ var warning = require('warning');
if (__DEV__) {
var reactProps = {
children: true,
defaultValue: true,
dangerouslySetInnerHTML: true,
key: true,
ref: true,
Expand Down Expand Up @@ -46,11 +47,10 @@ if (__DEV__) {
null
);

// For now, only warn when we have a suggested correction. This prevents
// logging too much when using transferPropsTo.
// Suggest a property name correction if possible
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like this is no longer a problem with #2532

Copy link
Contributor

Choose a reason for hiding this comment

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

It's still a problem; :%s/transferPropsTo/the spread operator/g.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack. Okay.

warning(
standardName == null,
'Unknown DOM property %s. Did you mean %s?',
'Unable to assign unsupported DOM property %s. Did you mean %s?',
name,
standardName
);
Expand All @@ -63,12 +63,20 @@ if (__DEV__) {
null
);

// Suggest an event name correction if possible
warning(
registrationName == null,
'Unknown event handler property %s. Did you mean `%s`?',
'Unable to register unsupported event handler property %s. Did you mean %s?',
name,
registrationName
);

// Otherwise at least make it clear that the attributes will be filtered out
warning(
standardName != null || registrationName != null,
'Unable to assign unsupported DOM property %s.',
name
);
};
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the warning copy here to make it clearer what's going on. Happy to change this.

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is that this error message doesn't specify the line number, as per the last sentence of my comment in #6459 (comment)


Expand Down