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

use size-limit to track gzip size of the van #163

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

btakita
Copy link

@btakita btakita commented Oct 30, 2023

I did some work on the https://github.com/nanostores/nanostores library. The library author wrote the size-limit library to track the gzip size of the library as changes are made. It's been useful for contributors to optimize the size of the library & to track the size impact of changes.

npm test:size
# or
size-limit
devDependencies
	∋ size-limit
	∋ @size-limit/preset-small-lib

@btakita btakita changed the title use size-limit library to track gzip size of the library: use size-limit to track gzip size of the van Oct 30, 2023
@Tao-VanJS
Copy link
Member

Tao-VanJS commented Oct 30, 2023

In publish.sh, there is a step to generate the .min.js.gz file along with the .min.js file (although the .min.js.gz is ignored by git thus not part of the repo):

gzip -kf ../public/van-$VER.min.js

Every release (indeed for every meaningful change to van.js), we're looking at the size of .min.js.gz file to assess the size impact.

I am not sure whether the extra benefit of size-limit library could outweigh the overhead of adding one more dev dependency.

@btakita
Copy link
Author

btakita commented Oct 30, 2023

It's beneficial for contributors to have an automated test suite that runs before commits. Something like size-limit hooks into the test suite to ensure that the gzip size is not exceeded. Notice that the current size is in package.json under the size-limit prop. The commit author can increase/decrease the size-limit as needed. This encodes the current size into the project commit history, so we have a record of size changes in the commit history.

@btakita
Copy link
Author

btakita commented Oct 30, 2023

I'm not concerned either way what tech is used. But just wanted to point out the utility of the DX.

@Tao-VanJS
Copy link
Member

Thanks @btakita for introducing the benefit! However, I noticed that the current limit in the package.json file (1009B) does not align with the size of the .min.js.gz file that I'm tracking (which is 970B at the moment). Maybe size-limit is using a different way of minifying and compressing JavaScript code that results in slightly larger size.

I'm okay with manually tracking the size changes for now (in every release announcement, I talked about the size impact if there is any). If there are tools that help automate the process, I'm open to it, but the result from the tool needs to align with the actual size we're getting for VanJS.

@btakita
Copy link
Author

btakita commented Oct 31, 2023

I think the npm script will have to run the build & call size-limit on the build output.

	devDependencies
		∋ size-limit
		∋ @size-limit/preset-small-lib

publish.sh: fix: build on linux systems
@btakita
Copy link
Author

btakita commented Oct 31, 2023

I updated the branch to run size-limit on public/van-latest.min.js. The size is 962 B. npm test now runs (cd ./src; ./publish.sh) && npm run test:size

publish.sh did not work on my Arch system so I updated the script to work on Linux. Hopefully it's compatible w/ OSX.

@@ -1,4 +1,4 @@
import van from "./van-1.2.5.js"
import van from "./van.js"
Copy link
Member

Choose a reason for hiding this comment

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

I think this change is a bug?

@@ -1,3 +1,4 @@
"use strict";
Copy link
Member

Choose a reason for hiding this comment

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

What caused the change here?

@@ -16,7 +16,8 @@
},
"type": "module",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
"test": "(cd ./src; ./publish.sh) && npm run test:size",
Copy link
Member

Choose a reason for hiding this comment

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

Could you change it to build? The command doesn't run any browser-based tests. Thus the name test could be misleading.

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.

2 participants