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

FS-23: Browser console errors from package updates #55

Merged
merged 2 commits into from
Feb 21, 2020

Conversation

russom-woldezghi
Copy link
Contributor

@russom-woldezghi russom-woldezghi commented Feb 11, 2020

Ticket

FS-23: Browser console errors from package updates

Description

  • Addressing browser console errors for componentWillReceiveProps
  • I replaced two warnings coming from the federated search by using componentDidUpdate for following components: 

    • FederatedRangeFacet (when the date range facet is cleared)
    • FederatedTextSearchNoAutocomplete (applies when autocomplete is enabled in .env.local.js)

And used prefixed UNSAFE_ for:

  • For FederatedTextSearchAsYouType component in federated search, I prefixed the lifecycle with a UNSAFE_, as updating the lifecycle to componentDidUpdate caused a fatal nesting error on render. We might need to open a new ticket to address this independently, there could additional work around this upgrade. There is a roadmap that will eventually drop support/usages of componentWillReceiveProps in React 17. I will need to dig into this separately as there could be additional work for updating FederatedTextSearchAsYouType (without appending UNSAFE_)

There are warnings coming from react-dates and react-autosuggest. These warning relates to Reacts update/new approach to implementing lifecycles. Seems like componentWillReceiveProps is being deprecated. The easiest way to suppress the warning is to prefix the lifecycle with a UNSAFE_.

The packages that have not upgraded their code to React 16 stated it will be a breaking change for the package. These are:

Additionally, when enabling autocomplete, I got a breaking checker function error in the console. I added PropTypes.shape{()} to the results and term properties for FederatedTextSearchAsYouType component.

For the packages using componentWillReceiveProps, we could roll a patch to add UNSAFE_ prefixes? Let me know if that makes sense as a temporary stop gap.

Testing

  • Pull down code
  • Remove nodes_modules
  • Ensure your env.local.js has autocomplete uncommented
  • Run yarn && yarn start
  • Perform searches, facet filtering on all the fields visible.
  • Ensure you only see warnings from DateInput and Autosuggest packages (more in Description, above) about this warning.
  • Observe for any other warning, asides from DateInput and Autosuggest packages.

@russom-woldezghi russom-woldezghi changed the title Browser console errors from package updates FS-23: Browser console errors from package updates Feb 11, 2020
@agentrickard
Copy link
Contributor

I suppose I would patch them for now. Everything else seems fine.

@russom-woldezghi
Copy link
Contributor Author

@agentrickard I added patches for react-autosuggest and react-dates packages to address the warnings componentWillReceiveProps (adding prefix of UNSAFE_).

Copy link
Contributor

@agentrickard agentrickard left a comment

Choose a reason for hiding this comment

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

This is working fine. What is the best way to merge this into 3.x as well?

@russom-woldezghi
Copy link
Contributor Author

@agentrickard I think this can be merged, if this looks good to you. I didn't notice any potential merge conflicts with the PR #56. I will follow-up on the warnings you posted for #56.

@agentrickard agentrickard merged commit eff5fed into 2.x Feb 21, 2020
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.

2 participants