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

feat: Lite - significantly reduce the size of tedious #1156

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

kristoffer-zliide
Copy link

@kristoffer-zliide kristoffer-zliide commented Sep 23, 2020

We are building a modern serverless architecture on a Microsoft cloud platform but struggling a bit with cold starts. We have experienced and read about others who have experienced a correlation between code size and startup times, when we realized that tedious has increased in size quite significantly since around version 4. Specifically, we see an increase in minified size of an app that does (almost) nothing except from issuing a SQL query from around 60kb to 1,900kb.

This pull request introduces a "tedious lite" alternative entry point that includes a lot less code that users who do not need "extra" things like exotic code pages and federated authentication can use to reduce their code size. This is done as a non-breaking change. Switching from normal tedious to tedious lite takes the minimal app size from around 1,900kb to around 530kb.

Future improvements could include handling date/time without joda, which would take the code size down even further to around 340kb.

Feedback welcome.

@IanChokS
Copy link
Member

Thanks for this PR! We'll take a look at this as soon as possible

@arthurschreiber
Copy link
Collaborator

This is a great idea! Not loading dependencies that are not always required will help application startup times a lot, and is definitely something worth improving.

I do think we'll want to take a slightly different approach, namely making use of dynamic import calls to load some of the modules dynamically.

This should be a straightforward change. @IanChokS is this something you want to work on?

@kristoffer-zliide
Copy link
Author

Thanks, @arthurschreiber! I just re-read what I wrote, I'm sure I meant that including tedious takes the size from 60kb to 1,900kb, not from 600kb to 1,900kb.

Anyways, we actually employ bundling (currently using browserify) to remove a lot of cruft in node_modules and do an overall minification. The 1,900kb number is also the minified size. Doing dynamic imports tend to trip that up. I think that's why I went with the alternate entry point.

@arthurschreiber
Copy link
Collaborator

@kristoffer-zliide You're right - dynamic imports might not actually solve much for the use case you have. I'll think about this a bit more.

@codecov
Copy link

codecov bot commented Jan 5, 2021

Codecov Report

Attention: Patch coverage is 56.81818% with 19 lines in your changes are missing coverage. Please review.

Project coverage is 79.33%. Comparing base (934e98e) to head (7acfb98).

❗ Current head 7acfb98 differs from pull request most recent head f92bd08. Consider uploading reports for the commit f92bd08 to get more accurate results

Files Patch % Lines
src/decoder.ts 21.05% 15 Missing ⚠️
src/errors.ts 82.60% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1156      +/-   ##
==========================================
+ Coverage   79.19%   79.33%   +0.14%     
==========================================
  Files          93       88       -5     
  Lines        4860     4477     -383     
  Branches      933      823     -110     
==========================================
- Hits         3849     3552     -297     
+ Misses        707      658      -49     
+ Partials      304      267      -37     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kristoffer-zliide
Copy link
Author

@arthurschreiber: did you think about it a bit more?

@kristoffer-zliide kristoffer-zliide changed the title Lite feat: Lite - significantly reduce the size of tedious Dec 30, 2022
@trgwii
Copy link

trgwii commented Jan 25, 2023

This is an excellent effort. In general I think mssql + tedious have become too bloated, and introducing alternatives for lighter modes or splitting out functionality into plugins might be a lot nicer in the future.

Here are some stats from my local tests:
Install of mssql in an empty project, no other dependencies: node_modules becomes 36.5MB
Install of tedious in an empty project, no other dependencies: node_modules becomes 36.0MB

Here's a breakdown of the worst offenders when installing tedious only:
image

@kristoffer-zliide
Copy link
Author

Yeah, I also experimented with removing joda ☺️

@trgwii
Copy link

trgwii commented Feb 13, 2023

I find myself continuously rechecking this issue for updates. In one of my applications at work, the mssql dependency and its dependencies makes up 71% of the size of the entire app including all dependencies.

@kristoffer-zliide
Copy link
Author

I was thinking a bit more about this. One thing is the shear size of the (minified) code, another is really the number of dependencies, which in increases the need for frequently updating packages and increases the number of issues reported by npm audit (which used to be a lot, but maybe not as much recently).

What if we took the connection-lite.ts from this PR and all the code it depended on, and all the packages it depends on and turned into a new package (perhaps called 'tedious-core') and published that. That package would change it's dependencies a lot less, would not need to be touched or updated when Azure-specific changes are made, and it wouldn't be impacted be audit issues in Azure packages and their dependencies, and, of course, it would be much smaller, both as deployed minified code, but also in dev's node_modules. The 'tedious' package could then depend on and extend 'tedious-core' in the way that connection.ts uses connection-lite.ts in this PR.

@arthurschreiber, @IanChokS: what do you think?

@arthurschreiber
Copy link
Collaborator

@kristoffer-zliide That sounds good 👍 But it will require some re-architecting of library repo - I'd like us to keep all the code for the different packages in one repo. I guess npm workspaces could help with that? But I've never used that.

@MichaelSun90
Copy link
Contributor

@kristoffer-zliide , Sorry that that I closed this earlier by mistake, and Thank you for putting the effort and thoughts on this idea.

Reduces a minified code size from ~1.4MB to ~850kb.
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.

5 participants