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

Make Polaris StrictMode compliant #785

Closed
jineshshah36 opened this issue Dec 19, 2018 · 4 comments
Closed

Make Polaris StrictMode compliant #785

jineshshah36 opened this issue Dec 19, 2018 · 4 comments
Milestone

Comments

@jineshshah36
Copy link
Contributor

Issue summary

Currently, there are many components that are not StrictMode compliant within Polaris because they using the legacy context API or use deprecated lifecycle methods. Given that React 16.7 will support a concurrent mode which should hopefully improve application performance for most situations, it would be great if Polaris because StrictMode compliant so that we could use it in the future without issue.

Expected behavior

Polaris should be StrictMode compliant

Actual behavior

Polaris is not StrictMode compliant

Steps to reproduce the problem

  1. Wrap Polaris components with the <React.StrictMode></React.StrictMode> component
  2. The console will begin spitting out errors.

Reduced test case

N/A

Specifications

N/A

@mbaumbach
Copy link
Contributor

mbaumbach commented Dec 20, 2018

Related issue: #457. Also, React 16.7 was just released and doesn't include concurrent mode (it's now in an unannounced future version), but agreed, it would be great to see Polaris compliant with StrictMode. From the comment in #457, it sounds like this may be a 4.0.0 feature.

@BPScott
Copy link
Member

BPScott commented Dec 20, 2018

Also related: #519 is an issue to track the removal of deprecated lifecycle hooks

@AndrewMusgrave
Copy link
Member

Hi @jineshshah36 , as Ben mentioned we're currently in the process of removing deprecated lifecycle hooks as tracked in #519. We don't have an issue tracking the switch to the new context api, but we've already begun using the new context api in a few components and will be switching more in the future.

@jineshshah36
Copy link
Contributor Author

I'm getting a 500 error when I try to visit #519, but that is good to hear

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

6 participants