-
Notifications
You must be signed in to change notification settings - Fork 46
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
Adding a .npmignore file and other ways to reduce the package size? #1220
Comments
Hi,
Can you give some examples of files you would prefer to not be included? Lines 13 to 20 in 62ca99b
See: https://docs.npmjs.com/cli/v8/configuring-npm/package-json#files As you can check in https://unpkg.com/browse/[email protected]/ there are no .ts source files, only .d.ts files which are typings that make the SDK easier to be using by TypeScript projects.As you can see we are only really adding the DOCUMENTATION.md on top of the sources, typings & published-by-npm-defaults files.
Our CI system (BalenaCI) is responsible for running
Yes, by default the SDK uses the 2015 build. In order to use a different build you can either use deep imports, or preferably use the
The
Regarding your graph, are you using a multistaged build, packing all sources into bundles and tree shaking away unused code? This for example would remove the browser build which is also found in the files published to npm. I've also opened this issue to stop publishing the unminified browser build, since that was 1.88MB on its own atm and there is one for both ES build targets. |
Connects-to: #1220 Change-type: patch Signed-off-by: Thodoris Greasidis <[email protected]>
Connects-to: #1220 Change-type: patch Signed-off-by: Thodoris Greasidis <[email protected]>
Thanks for this extensive reply! I am used to people seeing my KB chasing and nudging it down the todo list. Indeed, the typings are useful, and separating them in to an @types and an optional install for when developing may not be worth it for the saving of 1mb.
This has enlightened me to a whole bunch of things, now I understand why the 2018 folder is there. From reading around the SDK it looked like there were no circumstances the 2018 folder would be used unless the SDK code is changed. On the README it simply states: Still seems a shame to have to carry unused content, but off the top of my head I can't think of any silky way to separate them without breaking changes or more user friction, which for 3mb seems like it isn't worth it. Main thing is, I see now that the 2018 folder is not redundant. Shame those typings are needed, may speak to an advantage of having an @types system where the typings are added separately as needed, rather than bundling them all in to one package (when types are missing you usually get a nice prompt in Typescript too, including the install command required, so not huge user friction involved). Being able to go without the poly fills package would be nice, that is a big one, and presumably the balena-request package is used across other Balena products so could have a multiplier effect on cutting down package sizes. The graph may have been a bit confusing, it is just something http://bundlephobia.com kicked out. I was using the package on the backend, so not using tree shaking, and the graph is for bundling which is why the numbers are different to what I wrote about. Just thought it was interesting, but may be better suited for the other issue on the |
P.S. I had a quick run at the bundle approach to see how it would tree shake, but didn't get very far (Vite, Vue).
Might be because the url.resolve package is deprecated: It doesn't work when built either, but for a different reason. Browserify (https://github.com/browserify/browserify/blob/master/package.json) uses
And webpack 5: fkhadra/react-contexify#174 |
There is a bunch of stuff in the NPM release of the SDK that doesn't seem like it needs to be there. I was going to submit a pull request adding a .npmignore file. It seems like there used to be one but has been removed. I also couldn't find the build and deploy process to NPM in the repo, so wouldn't know where to put it/wouldn't be able to verify it would work.
Details of the npm ignore file can be found here: https://npm.github.io/publishing-pkgs-docs/publishing/the-npmignore-file.html
It could include typescript files which are currently included in the bundle:
That should bring the balena-sdk folder size down from
1mb
.Then there is two builds in the NPM package, the
es2015
and thees2018
. As far as I can tell from the code it doesn't use a.set
only a.get
which means it should always default to the the lowest supported version which is 2015? There isn't any need then to have the 2018 build included in the npm package? That would remove another2.9mb
, either by including the 2018 folder in the `.npmignore' or ideally changing the NPM build and deploy service not to build the 2018.In the packages.json file there are a bunch of @types included in the dependencies section (
4.6mb
), are they needed?:The
moment
package I opened a separate issue as it actually requires some coding, but that is another5.2mb
that could also help cut some weight (minus the size of whatever package/method replaces it).Another one is the
balena-request
package which is included in the production build section of the package.json file. I could only find it used in the tests folder and here but is10.1M
, a lot to do with theweb-streams-polyfill
which gets added at7.4mb
.Some of these of course would have implications for those developing the SDK based on the npm published package, but considering it is used for production and on IoT, would be good to try and get the size down a bit. This came up because the total package size is
40mb
, a whopper, and the things mentioned here account for23.8
of it.A necessary caveat, I haven't read the code here in the repo, just tracking down issues in my containers that are ballooning the sizes, and a lot points back to the SDK.
The text was updated successfully, but these errors were encountered: