search

keratin / authn-js

JavaScript client library for Keratin AuthN
GNU Lesser General Public License v3.0
45 stars 20 forks source link

reusable api as class instead of singleton #68

Open cornerman opened 7 months ago

cornerman commented 7 months ago

This makes the api part of AuthN reusable. AuthN itself would remain a singleton.

It actually does increase bundle size as @cainlevy already predicted.

First, I just added the API class using normal member methods, and I got the following results:

./lib/index.js → ./dist/keratin-authn.js...
┌──────────────────────────────────────────┐
│                                          │
│   Destination: ./dist/keratin-authn.js   │
│   Bundle Size:  15.39 KB                 │
│   Minified Size:  8.35 KB                │
│   Gzipped Size:  2.4 KB                  │
│                                          │
└──────────────────────────────────────────┘
./lib/index.js → ./dist/keratin-authn.min.js...
┌──────────────────────────────────────────────┐
│                                              │
│   Destination: ./dist/keratin-authn.min.js   │
│   Bundle Size:  7.4 KB                       │
│   Minified Size:  7.46 KB                    │
│   Gzipped Size:  2.34 KB                     │
│                                              │
└──────────────────────────────────────────────┘
./lib/index.js → ./dist/keratin-authn.module.js...
┌─────────────────────────────────────────────────┐
│                                                 │
│   Destination: ./dist/keratin-authn.module.js   │
│   Bundle Size:  14.9 KB                         │
│   Minified Size:  7.91 KB                       │
│   Gzipped Size:  2.3 KB                         │
│                                                 │
└─────────────────────────────────────────────────┘
created ./dist/keratin-authn.module.js in 55ms

Then I realized that the methods of API will break if we export them just like this, because of the this binding, so i used arrow functions:

./lib/index.js → ./dist/keratin-authn.js...
┌──────────────────────────────────────────┐
│                                          │
│   Destination: ./dist/keratin-authn.js   │
│   Bundle Size:  15.53 KB                 │
│   Minified Size:  8.19 KB                │
│   Gzipped Size:  2.4 KB                  │
│                                          │
└──────────────────────────────────────────┘
./lib/index.js → ./dist/keratin-authn.min.js...
┌──────────────────────────────────────────────┐
│                                              │
│   Destination: ./dist/keratin-authn.min.js   │
│   Bundle Size:  7.23 KB                      │
│   Minified Size:  7.29 KB                    │
│   Gzipped Size:  2.33 KB                     │
│                                              │
└──────────────────────────────────────────────┘
./lib/index.js → ./dist/keratin-authn.module.js...
┌─────────────────────────────────────────────────┐
│                                                 │
│   Destination: ./dist/keratin-authn.module.js   │
│   Bundle Size:  15.04 KB                        │
│   Minified Size:  7.74 KB                       │
│   Gzipped Size:  2.29 KB                        │
│                                                 │
└─────────────────────────────────────────────────┘

Then I thought, we could maybe safe some more bytes with arrow functions in the SessionManager:

./lib/index.js → ./dist/keratin-authn.js...
┌──────────────────────────────────────────┐
│                                          │
│   Destination: ./dist/keratin-authn.js   │
│   Bundle Size:  15.68 KB                 │
│   Minified Size:  7.97 KB                │
│   Gzipped Size:  2.38 KB                 │
│                                          │
└──────────────────────────────────────────┘
./lib/index.js → ./dist/keratin-authn.min.js...
┌──────────────────────────────────────────────┐
│                                              │
│   Destination: ./dist/keratin-authn.min.js   │
│   Bundle Size:  7.04 KB                      │
│   Minified Size:  7.1 KB                     │
│   Gzipped Size:  2.32 KB                     │
│                                              │
└──────────────────────────────────────────────┘
./lib/index.js → ./dist/keratin-authn.module.js...
┌─────────────────────────────────────────────────┐
│                                                 │
│   Destination: ./dist/keratin-authn.module.js   │
│   Bundle Size:  15.19 KB                        │
│   Minified Size:  7.53 KB                       │
│   Gzipped Size:  2.27 KB                        │
│                                                 │
└─────────────────────────────────────────────────┘

The current main is still a bit smaller though:

./lib/index.js → ./dist/keratin-authn.js...
┌──────────────────────────────────────────┐
│                                          │
│   Destination: ./dist/keratin-authn.js   │
│   Bundle Size:  14.15 KB                 │
│   Minified Size:  7.69 KB                │
│   Gzipped Size:  2.3 KB                  │
│                                          │
└──────────────────────────────────────────┘
./lib/index.js → ./dist/keratin-authn.min.js...
┌──────────────────────────────────────────────┐
│                                              │
│   Destination: ./dist/keratin-authn.min.js   │
│   Bundle Size:  6.39 KB                      │
│   Minified Size:  6.44 KB                    │
│   Gzipped Size:  2.15 KB                     │
│                                              │
└──────────────────────────────────────────────┘
./lib/index.js → ./dist/keratin-authn.module.js...
┌─────────────────────────────────────────────────┐
│                                                 │
│   Destination: ./dist/keratin-authn.module.js   │
│   Bundle Size:  13.65 KB                        │
│   Minified Size:  7.24 KB                       │
│   Gzipped Size:  2.2 KB                         │
│                                                 │
└─────────────────────────────────────────────────┘

fixes #67

cornerman commented 3 months ago

@cainlevy Anything speaks against merging this? Let me know, if we need to update something here.