sg-wireless / pymakr-vsc

GNU General Public License v3.0
99 stars 25 forks source link

Reduce extention load time further by using bundling ⏩ #181

Closed Josverl closed 2 years ago

Josverl commented 2 years ago

I think the load lime can be reduced, as is also indicated by the warning during the build process:

This extension consists of 8326 files, out of which 5130 are JavaScript files. For performance reasons, you should bundle your extension.

https://aka.ms/vscode-bundle-extension

This is a further optimization of #161

Suggested approach is to use esbuild for bundling. Previously I have tried bundling though webpack, and failed in spectacular ways, perhaps ESBuild is better suited.

Note that this will likely require a few changes to the internal workers in pymakr, as indicated by the below warning.

 > src/board/shell-workers.js:72:27: warning: Using direct eval with a bundler is not recommended and may cause problems (more info: https://esbuild.github.io/link/direct-eval)
    72 │                 var list = eval(data)
jakobrosenberg commented 2 years ago

You beat me to my reply @Josverl here https://github.com/pycom/pymakr-vsc/issues/161 😅

I've thought about bundling the plugin before, though last time I ended up stripping unused packages and code instead, which reduced the size from 99mb to 1mb IIRC.

My preferred bundler would be Rollup, but the main issue is whether a reduction in milliseconds on a "rare" bindings update justifies the increased project complexity and potential bugs. That said, I'm definitely open to the idea (and a PR? 😇), but it's not a high priority at the moment.

Josverl commented 2 years ago

Agree, on the prio and approach. ill give it a try with some of the bundlers if I can find some time my concern is however that some of the bugs might be hard to find, and with no unit or integration tests that manual validation is the only way to find those.

perhaps we should also add a base test framework such as in the hello-test example

then individual PRs can add to that for the areas they cover.

jakobrosenberg commented 2 years ago

Yes, test coverage for a USB device is going to be a pain in the CI.

We have E2E / integration tests, but they're very barebone and at the moment just checks that PyMakr can start. I think they're based off the link you shared.