webpack #996
webpack #996
Conversation
| return await extension.activateInternal(ctx, perfStats); | ||
| } | ||
|
|
||
| exports.activate = activate; |
There was a problem hiding this comment.
I don't see this being imported anywhere. Are these used by AzureTools?
There was a problem hiding this comment.
Yes. Added comment.
There was a problem hiding this comment.
I mean, used by vscode
// Export for use by vscode
exports.activate = activate;
exports.deactivate = deactivate;
| *.vsix | ||
| .vscode-test | ||
| test-results.xml | ||
| dist |
There was a problem hiding this comment.
nit: all but these last two are already in the gitignore
| @@ -12,3 +12,17 @@ tslint.json | |||
| .github/** | |||
| test-results.xml | |||
| .azure-pipelines/** | |||
| node_modules/** | |||
| out/** | |||
There was a problem hiding this comment.
Yeah a lot of these are duplicates. Here's the alphabetized, de-duped list for the entire file:
.azure-pipelines/**
.github/**
.gitignore
.travis.yml
.vscode-test/**
.vscode/**
*.tgz
**/*.map
build
dist/test/**
grammar
gulp*
node_modules/**
out/**
package-lock.json
src/**
stats.json
test-results.xml
test/**
testOutput/**
tsconfig.json
tslint.json
webpack.config*
(I tried out this extension to help: https://marketplace.visualstudio.com/items?itemName=Tyriar.sort-lines)
extension.ts
Outdated
| export { getAllCommandsFromText, getCommandFromTextAtLocation } from './src/mongo/MongoScrapbook'; | ||
| export { rejectOnTimeout, valueOnTimeout } from './src/utils/timeout'; | ||
|
|
||
| // These use instanceof and therefore we need to make sure we're using the same version of the bson module in the tests as in the bundle, |
There was a problem hiding this comment.
Does this only apply to test code? If not, it makes me kinda nervous. What exactly is the problem with instanceof? I guess I'd rather find a fix for instanceof than single out every single use of it we run in to.
Is this at all related to the thing you did here? microsoft/vscode-docker#745 Either way don't forget to do that here :)
There was a problem hiding this comment.
Yes, tests only (updated comment). The issue is that the test code is completely separate from the extension bundle, so if we do a blind import from the tests, you get two different instances of the module and hence two difference classes with different identity. That's also why we're importing everything from extension.ts instead of importing individual files directly from the tests like we used to, because you'd end up with two different instances of the code (and hence extensionVariables isn't a singleton, etc.).
extension.js is separate from the test code and uses an internal webpack version of import, so it doesn't know about anything imported by code outside of it (something I wish we could change - I tried).
There was a problem hiding this comment.
Is this at all related to the thing you did here? microsoft/vscode-docker#745 Either way don't forget to do that here :)
No. The other fix will be in the shared version.
| @@ -126,9 +126,9 @@ | |||
| } | |||
There was a problem hiding this comment.
Can you run another update on this file? I still see some changes when I run npm install on my machine. (typescript changed from 2.9.2 to 3.2.2)
src/extensionVariables.ts
Outdated
| @@ -17,6 +18,7 @@ export namespace ext { | |||
| export let context: ExtensionContext; | |||
| export let outputChannel: OutputChannel; | |||
| export let reporter: ITelemetryReporter; | |||
| export let resourcesPath = path.join(__dirname, 'resources'); | |||
There was a problem hiding this comment.
You have resourcesPath in both constants and extensionVariables. Do we need both? Seems kinda confusing.
There was a problem hiding this comment.
Removed from extvars
NOTES: