Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

perf: remove asn1.js and use node-forge #166

Merged
merged 10 commits into from
Feb 26, 2020
Merged

Conversation

alanshaw
Copy link
Member

@alanshaw alanshaw commented Feb 14, 2020

We are already using asn1 from node-forge in the code base. This PR removes the asn1.js dependency and uses the code we already imported from node-forge instead.

It should remove ~20 KB (gzipped) from the bundle.

Screenshot 2020-02-14 at 15 34 06


https://tools.ietf.org/html/rfc7518

@vasco-santos
Copy link
Member

This seems to be failing on chrome 😢 I re-run both chrome jobs, but no luck! We need to investigate, because this would be a nice gain

@alanshaw
Copy link
Member Author

@vasco-santos aye, is still WIP, I’ve not even removed everything I need to in order to remove asn1.js yet!

I just submitted the PR for visibility and intent.

@alanshaw alanshaw marked this pull request as ready for review February 17, 2020 14:54
@alanshaw
Copy link
Member Author

PASS ./dist/index.min.js: 119.41KB < maxSize 155KB (gzip)

🎉

test/util.spec.js Outdated Show resolved Hide resolved
src/keys/ecdh-browser.js Outdated Show resolved Hide resolved
src/util.js Outdated Show resolved Hide resolved
@alanshaw
Copy link
Member Author

Should be ready for review now

@alanshaw
Copy link
Member Author

Looks like this is also working upstream at ipfs/js-ipfs#2785

Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

Great work! This looks great ❤️

Copy link
Member

@mkg20001 mkg20001 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jacobheun jacobheun left a comment

Choose a reason for hiding this comment

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

🎉 looks good!

@jacobheun jacobheun merged commit 00477e3 into master Feb 26, 2020
@jacobheun jacobheun deleted the perf/remove-asn1.js branch February 26, 2020 16:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants