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

fix RangeError in encoding BLOB #66

Merged
merged 2 commits into from
Jun 12, 2019
Merged

Conversation

gfx
Copy link
Member

@gfx gfx commented Jun 12, 2019

Resolve #65

@codecov-io
Copy link

codecov-io commented Jun 12, 2019

Codecov Report

Merging #66 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #66      +/-   ##
=========================================
- Coverage    96.9%   96.9%   -0.01%     
=========================================
  Files          15      15              
  Lines         905     904       -1     
  Branches      183     183              
=========================================
- Hits          877     876       -1     
  Misses         28      28
Impacted Files Coverage Δ
src/Encoder.ts 97.79% <100%> (-0.01%) ⬇️

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 aed070f...d16cfdc. Read the comment docs.

@@ -300,14 +301,11 @@ export class Encoder {
this.pos++;
}

writeU8v(...values: Array<number>) {
writeU8a(values: ArrayLike<number>) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

YES, I was not spend too much time in Encoder, but i was going to mention to you ...reset params usage. This is not good for performance. TS maps it into loops and additional array allocation from passed arguments.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it is not just slow but causes RangeError for a large array. Too bad.

@gfx gfx merged commit 12afac8 into master Jun 12, 2019
@gfx gfx deleted the fix_range_error_on_encoding_blob branch June 12, 2019 13:54
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.

[bug] Maximum call stack size exceeded
3 participants