tedious icon indicating copy to clipboard operation
tedious copied to clipboard

feat: Lite - significantly reduce the size of tedious

Open kristoffer-zliide opened this issue 4 years ago • 12 comments

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.

kristoffer-zliide avatar Sep 23 '20 14:09 kristoffer-zliide

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

IanChokS avatar Sep 29 '20 17:09 IanChokS

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?

arthurschreiber avatar Dec 21 '20 11:12 arthurschreiber

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.

kristoffer-zliide avatar Dec 21 '20 12:12 kristoffer-zliide

@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.

arthurschreiber avatar Dec 21 '20 12:12 arthurschreiber

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).

:exclamation: 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 :warning:
src/errors.ts 82.60% 2 Missing and 2 partials :warning:
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     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 05 '21 17:01 codecov[bot]

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

kristoffer-zliide avatar Oct 07 '22 09:10 kristoffer-zliide

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

trgwii avatar Jan 25 '23 10:01 trgwii

Yeah, I also experimented with removing joda ☺️

kristoffer-zliide avatar Jan 25 '23 14:01 kristoffer-zliide

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.

trgwii avatar Feb 13 '23 08:02 trgwii

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?

kristoffer-zliide avatar Sep 11 '23 09:09 kristoffer-zliide

@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.

arthurschreiber avatar Sep 14 '23 16:09 arthurschreiber

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

MichaelSun90 avatar Sep 14 '23 16:09 MichaelSun90