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

perf: remove jwk2privPem and jwk2pubPem #162

Merged
merged 1 commit into from
Feb 3, 2020

Conversation

alanshaw
Copy link
Member

@alanshaw alanshaw commented Feb 2, 2020

These two unused functions required us to import the whole of the node-forge PKI implementation when we only use some RSA stuffs.

I thought this was going to lead to a big saving but then I realised that libp2p-keychain requires pkcs7 from node-forge which brings in a bunch of other modules...

It saves 34KB, but that only translates to 2KB when minified and gzipped. Still it's something, and I've done the research work now so might as well submit it 🤷‍♂️


Before:

Screenshot 2020-02-02 at 22 28 44

After:

Screenshot 2020-02-02 at 22 28 54

These 2 unused functions required us to import the whole of the node-forge PKI implementation when we only use some RSA stuffs.
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.

I did a quick run through of the js-libp2p ecosystem modules and indeed nothing is using these methods currently.

This LGTM, while the methods are not used it is a breaking change to the api, I will note the breaking change in the PR squash.

@jacobheun jacobheun merged commit cc20949 into master Feb 3, 2020
@jacobheun jacobheun deleted the perf/remove-unused-jwk-conversions branch February 3, 2020 13:28
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.

3 participants