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

Library size has tripled since v2 #55

Open
OliverJAsh opened this issue Aug 24, 2020 · 12 comments
Open

Library size has tripled since v2 #55

OliverJAsh opened this issue Aug 24, 2020 · 12 comments

Comments

@OliverJAsh
Copy link

Any idea why this might be?

https://bundlephobia.com/result?p=http-status-codes@2.1.1

image

@prettymuchbryce
Copy link
Owner

Thanks for calling this out. We just recently released v2.0.0 where we added a number of new exports. Particularly StatusCodes and ReasonPhrases each with pretty extensive documentation.

We're also exporting some additional variables for backwards compatibility, and we changed getReasonPhrase to be O(1) which required us to define a map of the status codes.

All of this results in more lines of code.

I definitely think there is some low-hanging fruit available for us to get the bundle size down. One easy way would be to strip all comments in .js file and leave them only in the TypeScript definition. But I'm not sure if this would negatively affect anyone's tooling.

Definitely open to ideas and PRs around this.

@OliverJAsh
Copy link
Author

I'm seeing a 2 KB (gzip) increase on my minified webpack bundle—so that's after all comments are removed.

I think the new StatusCodes enum means we can no longer benefit from tree shaking—even if only one enum member is used, the whole enum will be bundled.

Perhaps we can find a way to achieve the ergonomics of StatusCodes (so it can be used as a TypeScript union type) whilst preserving tree shaking.

// status-codes.js
export const OK = 200;
export const NOT_FOUND = 200;

// index.js
import * as StatusCodes from './status-codes';
export type StatusCodes = StatusCodes[keyof StatusCodes]
export { StatusCodes }

I think something like that will work because TypeScript allows values and types to share a common name.

@prettymuchbryce
Copy link
Owner

prettymuchbryce commented Sep 12, 2020

I think I've made some progress here.

https://bundlephobia.com/result?p=http-status-codes@2.1.4-beta.4

This sort of boils down to:

Part of Module gzipped
StatusCodes enum 1.0 kB
ReasonPhrases enum 1.2 kB
Deprecated legacy codes 642.0 B
Getters (getReasonPhrase, getStatusCode) 1.2 kB

I will ship this in 2.1.4. There is probably more than can be done here as well.

@OliverJAsh
Copy link
Author

@prettymuchbryce WDYT to this idea? I can send a PR if you like the sound of it. #55 (comment)

@prettymuchbryce
Copy link
Owner

@OliverJAsh I tried to play around with your suggestion, but I couldn't find a way to implement it which kept the enum in tact.

If you can manage to make the tree-shaking better than it is now then I'm very interested. 😄

@OliverJAsh
Copy link
Author

OliverJAsh commented Sep 25, 2020

@prettymuchbryce I had a quick look but it looks like those files are generated and I'm not too familiar with ts-morph. In any case I can show you what the output should look like.

src/index.ts:

import * as _StatusCodes from './status-codes';
export const StatusCodes = _StatusCodes;
export type StatusCodes = typeof StatusCodes[keyof typeof StatusCodes];

src/status-codes.ts:

// Generated file. Do not edit
/**
 * Official Documentation @ https://tools.ietf.org/html/rfc7231#section-6.3.3
 *
 * The request has been received but not yet acted upon. It is non-committal, meaning that there is no way in HTTP to later send an asynchronous response indicating the outcome of processing the request. It is intended for cases where another process or server handles the request, or for batch processing.
 */
export const ACCEPTED = 202;
/**
 * Official Documentation @ https://tools.ietf.org/html/rfc7231#section-6.6.3
 *
 * This error response means that the server, while working as a gateway to get a response needed to handle the request, got an invalid response.
 */
export const BAD_GATEWAY = 502;
/**
 * Official Documentation @ https://tools.ietf.org/html/rfc7231#section-6.5.1
 *
 * This response means that server could not understand the request due to invalid syntax.
 */
export const BAD_REQUEST = 400;

… you get the idea!

Usage as a type or as a value:

declare const fn: (x: StatusCodes) => void;

fn(1); // error
fn(StatusCodes.ACCEPTED); // no error

The same idea could be applied to other modules like reason-phrases.ts.

Note however that this unfortunately does not tree shake properly in webpack v4 due to a bug. It's fixed in webpack v5 though, and it works today in Rollup.

If you wanted something that works today in webpack v4, we will have to provide a different API. If you're interested I can say a bit more about that.

@OliverJAsh
Copy link
Author

@prettymuchbryce Hey, do you have any thoughts on the above? This issue is stopping us from upgrading. I would love to help get it fixed 😄

@prettymuchbryce
Copy link
Owner

@OliverJAsh I did you what you suggested here.

#64

Everything seems to work fine. The type is no longer an enum, but it looks like the most recent versions of tsserver still understand these types well enough to autocomplete them. The fact that they are no longer technically enums is probably OK.

My bigger concern is that it seems to have made the file size issue even worse according to bundlephobia. Any ideas why that might be?

https://bundlephobia.com/result?p=http-status-codes@2.1.5-beta.1

@OliverJAsh
Copy link
Author

My bigger concern is that it seems to have made the file size issue even worse according to bundlephobia. Any ideas why that might be?

Probably because of this webpack v4 bug. It's fixed in v5 but bundlephobia hasn't upgraded to that yet: https://github.com/pastelsky/bundlephobia/blob/a6046721fad9aaac2a78d9e00ac2e3e71ebd43b1/package.json#L86.

I created my own tests here:

Importing from sub-module (simpler case)

2.1.4

import { StatusCodes } from "http-status-codes/build/es/status-codes";
console.log(StatusCodes.ACCEPTED);
console.log(StatusCodes.BAD_GATEWAY);

webpack v4 output: 1.36 KB gzipped.
webpack v5 output: 937 B gzipped.

2.1.5-beta.1

import * as StatusCodes from "http-status-codes/build/es/status-codes";
console.log(StatusCodes.ACCEPTED);
console.log(StatusCodes.BAD_GATEWAY);

webpack v4 output: 480 B gzipped.
webpack v5 output: 66 B gzipped.

(()=>{"use strict";console.log(202),console.log(502)})();

Importing from index/main

I was able to achieve identical results to the above test after I made this change to index.js:

-import * as _StatusCodes from './status-codes';
-import * as _ReasonPhrases from './reason-phrases';
+import * as StatusCodes from './status-codes';
+import * as ReasonPhrases from './reason-phrases';
 export { getStatusCode, getReasonPhrase, getStatusText, } from './utils-functions';
-export var StatusCodes = _StatusCodes;
-export var ReasonPhrases = _ReasonPhrases;
+export { StatusCodes, ReasonPhrases }

We have to export the original namespace rather than an alias in order for webpack to understand export is a namespace and thus can be tree-shaken.

2.1.5-beta.1

import { StatusCodes } from "http-status-codes";
console.log(StatusCodes.ACCEPTED);
console.log(StatusCodes.BAD_GATEWAY);

webpack v5 output: 66 B gzipped.

(()=>{"use strict";console.log(202),console.log(502)})();

@RiZKiT
Copy link

RiZKiT commented Jan 24, 2023

All the above options did not work for me (with latest webpack and tercer plugin in production mode), I always get the whole object imported. Normally I never have missing tree shaking issues with this setup.

What works is import * as StatusCodes from "http-status-codes/build/es"; because it uses the legacy exports. My IDE also shows me in this case that e.g. StatusCode.OK is deprecated.

I will use this variant as long as there is no better solution.

@jschroeter
Copy link

It would be great if the individual status codes would be exported again to allow tree-shaking. We're using the lib also in client-side code and there it doesn't make much sense to ship all the status codes in the client bundle.

@prettymuchbryce
Copy link
Owner

It would be great if the individual status codes would be exported again to allow tree-shaking. We're using the lib also in client-side code and there it doesn't make much sense to ship all the status codes in the client bundle.

I am willing to accept a PR for this if anyone is interested in digging in here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants