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 #29933

Closed

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.

Everything looks good except the change to tsserverlibrary.d.ts. @weswigham can you take a look at that?

@@ -8347,7 +8347,7 @@ declare namespace ts.server {
excludedFiles: ReadonlyArray<NormalizedPath>;
private typeAcquisition;
updateGraph(): boolean;
getExcludedFiles(): readonly NormalizedPath[];
Copy link
Member

Choose a reason for hiding this comment

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

@weswigham I didn't expect to see this show up here.

Copy link
Member

Choose a reason for hiding this comment

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

Ahhhh, that's from the swap in gulp back to using the LKG for everything. The "remove jake" PR has the fix (accept the baseline, do a non-lkg validation build) already.

@@ -51,7 +51,7 @@ node_modules/async/autoInject.js(160,28): error TS2695: Left side of comma opera
node_modules/async/autoInject.js(164,14): error TS2695: Left side of comma operator is unused and has no side effects.
node_modules/async/autoInject.js(168,6): error TS2695: Left side of comma operator is unused and has no side effects.
node_modules/async/cargo.js(62,12): error TS2304: Cannot find name 'AsyncFunction'.
node_modules/async/cargo.js(67,14): error TS2591: Cannot find name 'module'. Do you need to install type definitions for node? Try `npm i @types/node` and then add `node` to the types field in your tsconfig.
node_modules/async/cargo.js(67,14): error TS2749: 'module' refers to a value, but is being used as a type here.
Copy link
Member

Choose a reason for hiding this comment

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

Seems like a regression from #29896, although I think maybe both errors should not happen in JS. Need to investigate.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, this is just horrible misunderstanding of the jsdoc module:QualifiedName syntax.

@@ -2,12 +2,16 @@ Exit Code: 1
Standard output:
node_modules/follow-redirects/index.js(105,10): error TS2339: Property 'emit' does not exist on type 'RedirectableRequest'.
node_modules/follow-redirects/index.js(106,10): error TS2339: Property 'abort' does not exist on type 'RedirectableRequest'.
node_modules/follow-redirects/index.js(173,10): error TS2339: Property 'emit' does not exist on type 'RedirectableRequest'.
Copy link
Member

Choose a reason for hiding this comment

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

looks like a new version of follow-redirects

Copy link
Member

Choose a reason for hiding this comment

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

@@ -282,7 +276,7 @@ node_modules/chrome-devtools-frontend/front_end/animation/AnimationModel.js(514,
node_modules/chrome-devtools-frontend/front_end/animation/AnimationModel.js(553,11): error TS2339: Property 'AnimationModel' does not exist on type '{ new (effect?: AnimationEffect, timeline?: AnimationTimeline): Animation; prototype: Animation; }'.
node_modules/chrome-devtools-frontend/front_end/animation/AnimationModel.js(583,43): error TS2339: Property 'remove' does not exist on type 'Map<string, AnimationGroup>'.
node_modules/chrome-devtools-frontend/front_end/animation/AnimationModel.js(665,37): error TS2339: Property 'AnimationModel' does not exist on type '{ new (effect?: AnimationEffect, timeline?: AnimationTimeline): Animation; prototype: Animation; }'.
node_modules/chrome-devtools-frontend/front_end/animation/AnimationModel.js(691,24): error TS2304: Cannot find name 'Image'.
Copy link
Member

Choose a reason for hiding this comment

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

Due to #29896. This one is an improvement.

@@ -19,12 +19,7 @@ node_modules/chrome-devtools-frontend/front_end/Runtime.js(77,16): error TS7014:
node_modules/chrome-devtools-frontend/front_end/Runtime.js(78,16): error TS7014: Function type, which lacks return-type annotation, implicitly has an 'any' return type.
node_modules/chrome-devtools-frontend/front_end/Runtime.js(95,28): error TS2339: Property 'response' does not exist on type 'EventTarget'.
node_modules/chrome-devtools-frontend/front_end/Runtime.js(147,37): error TS2339: Property '_importScriptPathPrefix' does not exist on type 'Window'.
node_modules/chrome-devtools-frontend/front_end/Runtime.js(158,21): error TS2345: Argument of type 'Promise<string>' is not assignable to parameter of type 'Promise<undefined>'.
Copy link
Member

Choose a reason for hiding this comment

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

interesting. how did generic assignability change? the code looks like this:

oh. it looks like then's type got better at inferring result types, so it's correctly undefined now.
So...one of #29847, #29787 or #29858 then? Either way, it's an improvement.

@weswigham weswigham closed this Feb 21, 2019
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.

3 participants