Fixed repository URLs: Publish

[fuhlig]

The incorrect repository URLs were fixed in https://github.com/facebook/regenerator/commit/2bd87d609a899f0b3157382bb20d8e9275463015

Since the fix is not yet published, this can lead to broken links, e.g. when creating third party license references.

Please publish the fix to npm. Thanks in advance.

[ljharb]

Browsers don't offer the API for "object that opens on click" embedded in the message in a reliable cross-browser way, and what would happen on node?

[goldylucks]

object that opens on click is default behavior on chrome. it's not like that cross library?

If we can console everything as independent arguments instead of a string, wouldn't it work?

Also here are some ideas for a first step to check the validity of this direction:

1. json strongify the entire component's instance props and append/prepend that to the warning message
2. try to find a common identifier prop name by iterating over the props, i.e. id || name || title || text
3. allow defining something like static missingPropTypeIdentifier = id on the class
4. allow defining something like `PropTypes.configure({ missingPropTypeIdentifiers: ["id", "name", "title", "text"]

what do you think?

I'm doing this currently in my code and so far it works great:

I have a list of hundreds of restaurants, and immediately I know which one is missing the prop. Here's the code for that:

import PropTypes from 'prop-types'

// eslint-disable-next-line import/prefer-default-export
export const reportMissingProps = (Component, componentInstance) => {
  if (!Component.propTypes) {
    console.warn('[Report missing props] -> propTypes empty -> bailing')
    return
  }
  // see https://github.com/facebook/react/issues/16069
  PropTypes.checkPropTypes(
    Component.propTypes,
    componentInstance.props,
    'prop',
    `${Component.name} -> ${identifyingProps(componentInstance)}`
  )
}

function identifyingProps(componentInstance) {
  const {
    props: { id, name, title },
  } = componentInstance
  return id || name || title
}

What do you think @ljharb ?

[ljharb]

It's definitely not like that in every browser, and it's definitely not like that in node, which is text only.

Separately, not every browser (or every supported version of every browser) supports more than one argument to console.log.

What I'm confused about is how you have so many instances of the same component. but that aren't defined in the same place in code (ie, in a .map or similar). What does your code look like that it's even possible to omit passing the prop to just one instance?

[goldylucks]

It's definitely not like that in every browser, and it's definitely not like that in node, which is text only.

Gotcha, thanks for educating me on this, I forget sometimes there's a world outside of chrome :man_facepalming:

Separately, not every browser (or every supported version of every browser) supports more than one argument to console.log.

That's perfectly fine, we can print everything directly as a string in the console. We can also call console.log multiple times.

What does your code look like that it's even possible to omit passing the prop to just one instance?

I get some of the data from an external API, which has hiccups. So I get back an array of hundreds of items, and one of them is missing the latitude property. Makes sense?

[ljharb]

We can't call console.log more than once for a single error; that would pollute the log for people.

It sounds like what you really need is to validate the data before passing it into the react tree - using something like jsonschema, perhaps?

[goldylucks]

indeed, I can use many strategies to validate data on my end. But why not improve the developer experience of all React's users?

Do you see any drawbacks in printing extra 3-6 characters in the prop warning in order to identify the instance?

[ljharb]

I'm claiming that it's not in fact all of React's users - that typically, either the API has correct data, or everyone validates it.

I see drawbacks in adding complexity and performance overhead.

[goldylucks]

I can't imagine I'm the only developer who stumbled upon this, but who knows.

performance overhead

why is that? it's only in development, and only when there's an error to report, otherwise the extra function call to print the identifier won't be called.

[ljharb]

Development has to perform well too.

[goldylucks]

only when there's an error to report

so there's no impact to development unless there are prop type errors, in which case the developer will fix it anyway.

So how does this hurts performance?

[ljharb]

Prop type errors don’t interrupt rendering, by design - devs don’t have to fix them and their app needs to keep working the same.

[asbjornh]

Using arbitrary keys like id, name and so on seems a bit weird, and so does dumping JSON strings or object literals to the "console". But maybe outputting the key (when it exists) directly in the error message could help without adding too much overhead?

Warning: Failed prop type: The prop 'blabla' is marked as required in 'SomeComponent' with key 'abc', but its value is 'null'.

[goldylucks]

Sounds good!

On Mon, Aug 5, 2019, 13:13 Asbjørn Hegdahl [email protected] wrote:

Using arbitrary keys like id, name and so on seems a bit weird, and so does dumping JSON strings or object literals to the "console". But maybe outputting the key (when it exists) directly in the error message could help without adding too much overhead?

Warning: Failed prop type: The prop 'blabla' is marked as required in 'SomeComponent' with key 'abc', but its value is null.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/facebook/prop-types/issues/279?email_source=notifications&email_token=ABVEADAE5R6URI6VPCUY33LQC74ODA5CNFSM4H6SUV62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3RLT3Y#issuecomment-518175215, or mute the thread https://github.com/notifications/unsubscribe-auth/ABVEADDKWEYOXOT2W766J73QC74ODANCNFSM4H6SUV6Q .

[ljharb]

@asbjornh if you want to try submitting a PR, that seems like it might be worth exploring.

[asbjornh]

@ljharb I made a draft PR because I'm not feeling super confident about the idea anymore