Skip to content
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

Conversation

typescript-bot
Copy link
Collaborator

Please review the diff and merge if no changes are unexpected.
You can view the build log here.

cc @weswigham @sandersn @RyanCavanaugh

Copy link
Member

@sandersn sandersn left a 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.
Copy link
Member

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.

Copy link
Member

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 ?

Copy link
Member

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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they added code.

@sandersn
Copy link
Member

Opened vpulim/node-soap#1059

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants