microsoft / vscode-python

Python extension for Visual Studio Code
https://aka.ms/pvsc-marketplace
MIT License
4.34k stars 1.19k forks source link

Exclude *.map files from shipped extension #13961

Open mjbvz opened 4 years ago

mjbvz commented 4 years ago

Extension Version: 2020.8.109390

I was looking into why Python was taking a little while to download on my machine. From a quick investigation, it looks like the datascience-ui folder in the shipped extension contains three large .map files:

du -ah . | sort -rh | grep '\.map'
14M ./out/client/extension.js.map.disabled
7.1M    ./out/datascience-ui/notebook/commons.initial.bundle.js.map
3.4M    ./out/datascience-ui/viewers/commons.initial.bundle.js.map
216K    ./out/datascience-ui/notebook/nativeEditor.js.map
128K    ./out/datascience-ui/notebook/interactiveWindow.js.map
8.0K    ./out/datascience-ui/viewers/plotViewer.js.map
8.0K    ./out/datascience-ui/viewers/dataExplorer.js.map

These should usually be excluded as they are only needed during development

mjbvz commented 4 years ago

Although these files are explicitly allowed in the vscodeignore so this seems intentional: https://github.com/microsoft/vscode-python/blame/main/.vscodeignore#L4

Still these are around 1/5 of the extensions's side on disk so just want to confirm these are really required for the extension to work

karrtikr commented 4 years ago

Hey @mjbvz It's actually necessary, we have a setting which uses that logic. I'll post more details as soon as I can.

DonJayamanne commented 4 years ago

We ship source maps so we get the exact line numbers in our code. This was crucial for instances when we had unhandled exceptions. Back in the early days we used to rename the source maps files as it used to adversely impact loading of the extension. I.e. if we had a .js.map file, then extension loading times was very slow, hence we decided to rename it to `.js.map.disabled`.

I tested this yesterday & couldn't find any per degradation anymore. I guess we can just ship it as is. As for the source maps for the datascience-ui, we need that again to determine the exact line & file that threw an exception.

FYI - common.initial.bundle.js was excluded, as these this source map file contained source maps for 3rd party depedencies from npm packages, and we didn't need them, hence they were excluded. Looks like they have crept back into the extension.

DonJayamanne commented 4 years ago

@rchiodo @IanMatthewHuff Do we need the source maps for the DS ui code?

rchiodo commented 4 years ago

No I don't think so. We only need them when debugging, not when getting logs from users.

DonJayamanne commented 4 years ago

Summary

karrtikr commented 4 years ago

I set "python.diagnostics.sourceMapsEnabled": true in user settings, and activated the marketplace extension. I get Failed to initialize source map support in extension message and I don't get exact file and line numbers.

image

But when debugging the extension, I still get Failed to initialize source map support in extension, but get exact line & file which threw the errors.

image

@DonJayamanne Is this a bug? Is it only meant to work when debugging? I think source maps is not working for extensions in the marketplace.

luabud commented 4 years ago

We should:

If we need this for debugging, we can give users a link to the map files and instructions as to how to put it on the right place, so we get the info we need