search

balena-io / balena-sdk

The SDK to make balena powered JavaScript applications.
https://www.balena.io/
Apache License 2.0
146 stars 46 forks source link

Adding a .npmignore file and other ways to reduce the package size? #1220

Open maggie44 opened 2 years ago

maggie44 commented 2 years ago

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:

*.ts
*.d.ts
**/typings
**/types

That should bring the balena-sdk folder size down from 1mb.

Then there is two builds in the NPM package, the es2015 and the es2018. 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 another 2.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?:

    "@types/json-schema": "^7.0.9",
    "@types/lodash": "^4.14.168",
    "@types/memoizee": "^0.4.5",
    "@types/node": "^10.17.55",

The moment package I opened a separate issue as it actually requires some coding, but that is another 5.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 is 10.1M, a lot to do with the web-streams-polyfill which gets added at 7.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 for 23.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.

Screenshot 2022-04-24 at 22 04 29
thgreasi commented 2 years ago

Hi, Thanks for reaching about this.

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.

Can you give some examples of files you would prefer to not be included? We are no longer using a .npmignore file, since we rely on the package.json's files field, to define exactly the files that we think should be part of the published npm package. See: https://github.com/balena-io/balena-sdk/blob/62ca99bd0ffc0153110c55ec3d62561f8072a367/package.json#L13-L20 See: https://docs.npmjs.com/cli/v8/configuring-npm/package-json#files As you can check in https://unpkg.com/browse/balena-sdk@16.19.8/ 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.

I also couldn't find the build and deploy process to NPM in the repo...

Our CI system (BalenaCI) is responsible for running npm test and npm publish when a PR gets merged.

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?

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 balena-es-version to .set the ES version that you would prefer all your compatible dependencies to be built for. See: https://github.com/balena-io-modules/balena-es-version

In the packages.json file there are a bunch of https://github.com/types included in the dependencies section (4.6mb), are they needed?:

  • @types/json-schema is required since an exported type is extending it in See: https://github.com/resin-io/resin-sdk/blob/62ca99bd0ffc0153110c55ec3d62561f8072a367/lib/models/config.ts#L46
  • @types/lodash is indeed not used. Nice spot :+1: PRing in a bit.
  • @types/memoizee this is used in two internal functions in models/os.ts but I will open a PR to no longer re-export the memoized types, which should allow us to move the memoizee types to the "devDependencies".
  • @types/node I'm not really sure about this and will have to further investigate.

The balena-request package is handling all requests of the SDK to our backend, including log streaming, which is what we need the web-streams-polyfill for. Thanks for the heads-up though, since at this point we might be able to no longer rely on that polyfill.

my containers that are ballooning the sizes

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. See: https://github.com/balena-io/balena-sdk/issues/1222

maggie44 commented 2 years ago

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.

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 balena-es-version to .set the ES version that you would prefer all your compatible dependencies to be built for. See: https://github.com/balena-io-modules/balena-es-version

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: You can also use the es2018 version if desired.. It may be worth including your paragraph above in there, it would likely have pointed me in the right direction.

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 moment package.

maggie44 commented 2 years ago

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

import { getSdk } from 'balena-sdk'

const sdk = getSdk()
void test()
async function test() {
  await sdk.auth.loginWithToken('key')
  await sdk.models.device.get('uuid')
}
[Error] TypeError: url.resolve is not a function. (In 'url.resolve(backendParams.apiUrl, `/${backendParams.apiVersion}/`)', 'url.resolve' is undefined)
    PinejsClient (balena-sdk.js:31848)
    createPinejsClient (balena-sdk.js:31879)
    getSdk (balena-sdk.js:40527)
    Module Code (App.vue:4)
    evaluate
    moduleEvaluation
    moduleEvaluation
    moduleEvaluation
    (anonymous function)
    promiseReactionJob

Might be because the url.resolve package is deprecated:

Screenshot 2022-04-28 at 11 13 00

It doesn't work when built either, but for a different reason.

Browserify (https://github.com/browserify/browserify/blob/master/package.json) uses buffer (https://github.com/feross/buffer) which uses process variable which isn't supported in Vite (https://github.com/vitejs/vite/issues/1973)

Uncaught ReferenceError: process is not defined

And webpack 5: https://github.com/fkhadra/react-contexify/issues/174