Skip to content

🤖 User test baselines have changed #27048

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

Merged
merged 1 commit into from
Sep 12, 2018

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.

Couple of things need investigation before merging.

@@ -284,7 +284,7 @@ node_modules/chrome-devtools-frontend/front_end/animation/AnimationModel.js(811,
node_modules/chrome-devtools-frontend/front_end/animation/AnimationModel.js(811,44): error TS2300: Duplicate identifier 'Request'.
node_modules/chrome-devtools-frontend/front_end/animation/AnimationScreenshotPopover.js(7,11): error TS2339: Property 'AnimationScreenshotPopover' does not exist on type '{ new (effect?: AnimationEffect, timeline?: AnimationTimeline): Animation; prototype: Animation; }'.
node_modules/chrome-devtools-frontend/front_end/animation/AnimationScreenshotPopover.js(9,23): error TS2304: Cannot find name 'Image'.
node_modules/chrome-devtools-frontend/front_end/animation/AnimationScreenshotPopover.js(18,39): error TS2345: Argument of type 'new (width?: number, height?: number) => HTMLImageElement' is not assignable to parameter of type 'Node'.
node_modules/chrome-devtools-frontend/front_end/animation/AnimationScreenshotPopover.js(18,39): error TS2322: Type 'new (width?: number, height?: number) => HTMLImageElement' is not assignable to type 'Node'.
Copy link
Member

Choose a reason for hiding this comment

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

Did we get rid of the bad-parameter-specific error message? Seems odd.

Copy link
Member

Choose a reason for hiding this comment

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

Here's a repro:

declare var ohno: new () => HTMLImageElement;
declare function ff(t: Node): void;
ff(ohno)

I'll file a bug, but I'm not sure how bad this is.

Copy link
Member

Choose a reason for hiding this comment

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

Filed #27049
@weswigham can you decide if this is By Design or not?

Copy link
Member

Choose a reason for hiding this comment

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

Hm. When we elaborate we don't bother using the override message right now, since we assume we won't trigger the elaboration right at the top. We could pass is thru with a little depth tracking?

node_modules/debug/src/node.js(26,1): error TS2323: Cannot redeclare exported variable 'colors'.
node_modules/debug/src/node.js(31,5): error TS2323: Cannot redeclare exported variable 'colors'.
node_modules/debug/src/node.js(60,39): error TS2345: Argument of type 'string | undefined' is not assignable to parameter of type 'string'.
node_modules/debug/dist/debug.js(3,100): error TS2539: Cannot assign to '_typeof' because it is not a variable.
Copy link
Member

Choose a reason for hiding this comment

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

These changes are probably the result of a major new version but I need to confirm.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the code seems entirely different.

I'll accept these baselines for now, but I should probably sweep through at some point and make sure the new errors are mostly good. There's about 40 more than before.

@sandersn sandersn merged commit 371ffff into microsoft:master Sep 12, 2018
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