-
Notifications
You must be signed in to change notification settings - Fork 30
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
Throw an error for any item that resolves as undefined #825
Conversation
@@ -571,7 +571,7 @@ | |||
}, | |||
"scripts": { | |||
"vscode:prepublish": "npm run cleanReadme", | |||
"build": "tsc && gulp webpack-prod", | |||
"build": "tsc", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentionally included?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the npm run build
shouldn't run webpack. It wasn't actually changing the line return await extension.activate(ctx, perfStats, true);
either (where the true is ignoreBundle) so it wasn't actually prepping it to be used with webpack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can confirm that this matches our other extensions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We really should stop using Gulp and match the Docker extension's build scripts and such...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, agreed: #826
Mitigates microsoft/vscode-azurefunctions#3851 and other issues, I'm sure
We see this error floating around quite often. While this doesn't actually fix it, at least the error makes a little more sense to the user.