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

Refactor pattern of spreading empty arrays to Array.from #266

Merged
merged 6 commits into from
Apr 23, 2021

Conversation

corymharper
Copy link
Contributor

This pull request refactors all patterns within the hook resembling the following: [...Array(someInteger)].map(someValue) to follow a pattern resembling: Array.from({ length: someInteger }, someValue). This is due to a transformation that Docusaurus was using on top of the transformations made by Typescript when code was being compiled that caused unexpected behavior in the demo website, resulting in a crash.

@codecov
Copy link

codecov bot commented Apr 22, 2021

Codecov Report

Merging #266 (15bb24f) into master (3d7aa9f) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #266   +/-   ##
=======================================
  Coverage   91.86%   91.86%           
=======================================
  Files           1        1           
  Lines          86       86           
  Branches       24       24           
=======================================
  Hits           79       79           
  Misses          7        7           
Impacted Files Coverage Δ
src/use-dropdown-menu.ts 91.86% <100.00%> (ø)

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 3d7aa9f...15bb24f. Read the comment docs.

@WesCossick
Copy link
Member

Specifically, Docusaurus was unwrapping [...Array(someInteger)].map() into just Array(someInteger).map(), which does not behave the same. I believe Docusaurus uses webpack, so it's possible that projects that depend on this package could be affected by this as well.

@WesCossick
Copy link
Member

Per our conversation on Slack, be sure to bump the patch version number so that we can publish this bug fix.

@WesCossick WesCossick merged commit 09208ac into master Apr 23, 2021
@WesCossick WesCossick deleted the demo-site-crash-fix branch April 23, 2021 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants