-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
🤖 User test baselines have changed #31034
🤖 User test baselines have changed #31034
Conversation
002e4d1
to
8d61fff
Compare
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.
Changes are OK, but we should push a fix to since they are reasonably popular (92k/week) and I would like to keep them happy about their recent upgrade to Typescript.
@@ -0,0 +1,12 @@ | |||
Exit Code: 1 | |||
Standard output: | |||
node_modules/soap/lib/client.d.ts(4,26): error TS7016: Could not find a declaration file for module 'request'. '../../../tests/cases/user/soap/node_modules/request/index.js' implicitly has an 'any' type. |
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.
they upgraded to typescript, but not to --strict, so we're seeing an error that they are not.
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.
I'm not sure what the correct fix is here — it looks to me like they need @types/request
in their dependencies, not just devDependencies. Opinions @RyanCavanaugh or @weswigham ?
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.
dependencies
- it's the only way to get npm i
to install transitive dependencies for consumers (reasonably).
@@ -8,105 +8,105 @@ node_modules/uglify-js/lib/ast.js(870,14): error TS2339: Property 'push' does no | |||
node_modules/uglify-js/lib/ast.js(877,14): error TS2339: Property 'pop' does not exist on type 'TreeWalker'. | |||
node_modules/uglify-js/lib/ast.js(932,25): error TS2339: Property 'self' does not exist on type 'TreeWalker'. | |||
node_modules/uglify-js/lib/ast.js(933,37): error TS2339: Property 'parent' does not exist on type 'TreeWalker'. | |||
node_modules/uglify-js/lib/compress.js(174,42): error TS2554: Expected 0 arguments, but got 1. |
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.
they added code.
Opened vpulim/node-soap#1059 |
8d61fff
to
7908904
Compare
7908904
to
60e8124
Compare
Please review the diff and merge if no changes are unexpected.
You can view the build log here.
cc @weswigham @sandersn @RyanCavanaugh