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

Replace deepEqual with react-fast-compare #402

Merged
merged 1 commit into from Nov 16, 2018
Merged

Conversation

shad-k
Copy link
Contributor

@shad-k shad-k commented Sep 28, 2018

Fixes #373

@CLAassistant
Copy link

CLAassistant commented Sep 28, 2018

CLA assistant check
All committers have signed the CLA.

@tmbtech
Copy link
Contributor

tmbtech commented Nov 14, 2018

@shad-k looks like there is a failure in the build step. We will need to resolve before merging into master. I will have sometime tomorrow to look deeper into it. In the meantime, is it possible for you to merge master into your PR?

@codecov
Copy link

codecov bot commented Nov 15, 2018

Codecov Report

Merging #402 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #402   +/-   ##
======================================
  Coverage    97.6%   97.6%           
======================================
  Files           3       3           
  Lines         292     292           
======================================
  Hits          285     285           
  Misses          7       7
Impacted Files Coverage Δ
src/Helmet.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e04b667...e13db9c. Read the comment docs.

@shad-k
Copy link
Contributor Author

shad-k commented Nov 15, 2018

@tmbtech Done.

@tmbtech tmbtech merged commit 1803f67 into nfl:master Nov 16, 2018
@tadjohnston
Copy link

More out of curiosity, but what is the release timeline for this? The last time you guys published was over a year ago. I'm just wondering if it's pointless to implement the work around if you guys are releasing soon.

@tmbtech
Copy link
Contributor

tmbtech commented Nov 17, 2018

@tadjohnston.

I would like to merge #395 in first. Then we can publish a beta version for feedback.

Thoughts?

@rcohen-unext
Copy link

We are running into this issue now on our project as well so the sooner a release can get out the better.

@ahmedelgabri
Copy link

Agree with others here we also have the same issue, I think releasing this first makes more sense.

@catamphetamine
Copy link

Is there any ETA on the release of this fix?

@zubhav
Copy link

zubhav commented Nov 22, 2018

There seems to be a lot of projects depending on this fix. Can this be prioritised please?

@tmbtech
Copy link
Contributor

tmbtech commented Nov 25, 2018

Yup. Will address right after the holidays.

@tadjohnston
Copy link

Thanks @tmbtech! I think the cjs and esm builds are gonna be great, but if it we could get this patched first that would be ideal IMO.

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

Successfully merging this pull request may close these issues.

None yet

9 participants