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

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 @mhegazy

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Aug 1, 2018

There are really only 19 unique new issues, but they are clearly impactful and indicative of much broader breaks.

# Error message
0 Property 'reason' does not exist on type 'CancelToken'.
1 Duplicate identifier 'CSSRule'.
2 Duplicate identifier 'Comment'.
3 Duplicate identifier 'Event'.
4 Duplicate identifier 'Position'.
5 Duplicate identifier 'Request'.
6 Duplicate identifier 'Window'.
7 Property 'style' does not exist on type 'HTMLDivElement'.
8 Property 'style' does not exist on type 'HTMLElement'.
9 Property 'setImmediate' does not exist on type 'Window'.
10 Property 'logicalXDPI' does not exist on type 'Screen'.
11 Property 'deviceXDPI' does not exist on type 'Screen'.
12 Property 'logicalYDPI' does not exist on type 'Screen'.
13 Property 'deviceYDPI' does not exist on type 'Screen'.
14 Property 'offsetTop' does not exist on type 'Element'.
15 Property 'style' does not exist on type 'Icon'.
16 Type '[CSSStyleSheetHeader, any[]][]' is not assignable to type 'CoverageInfo[]'.
17 Type '(a: SortableDataGridNode, b: SortableDataGridNode) => number' is not assignable to type '(arg0: NODE_TYPE, arg1: NODE_TYPE) => number'.
18 Type 'NODE_TYPE[][]' is not assignable to type 'ViewportDataGridNode[][]'.

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.

Here's some of the reasons for the breaks. I think it's mostly (all?) down to the recent update to the generated d.ts files.

Exit Code: 1
Standard output:
node_modules/@types/react-native/index.d.ts(3895,26): error TS2693: 'setImmediate' only refers to a type, but is being used as a value here.
node_modules/@types/react-native/index.d.ts(3896,28): error TS2693: 'clearImmediate' only refers to a type, but is being used as a value here.
Copy link
Member

Choose a reason for hiding this comment

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

I am really unsure what is going on here. setImmediate and clearImmediate come from @types/node and shouldn't have anything in the type space because they are functions (although setImmediate has a merged namespace, it's a value namespace).

Copy link
Member

Choose a reason for hiding this comment

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

If there's no @types/node, then it could be coming from dom.generated.d.ts, which did delete a bunch of references recently.

@@ -26,6 +26,9 @@ lib/axios.js(41,7): error TS2339: Property 'CancelToken' does not exist on type
lib/axios.js(42,7): error TS2339: Property 'isCancel' does not exist on type 'Axios'.
lib/axios.js(45,7): error TS2339: Property 'all' does not exist on type 'Axios'.
lib/axios.js(48,7): error TS2339: Property 'spread' does not exist on type 'Axios'.
lib/cancel/CancelToken.js(23,15): error TS2339: Property 'reason' does not exist on type 'CancelToken'.
lib/cancel/CancelToken.js(28,11): error TS2339: Property 'reason' does not exist on type 'CancelToken'.
lib/cancel/CancelToken.js(29,26): error TS2339: Property 'reason' does not exist on type 'CancelToken'.
Copy link
Member

Choose a reason for hiding this comment

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

this is now typed because CancelToken has an @class tag.

Note that this, and the existing two errors, is a result of not tracking this property assignments through aliasing:

var token = this
executor(function (message) {
  if (token.reason) { return }
  token.reason = new Cancel(message)
})

If you switch to an arrow function and remove the aliasing, we infer this correctly:

executor((message) => {
  if (this.reason) { return }
  this.reason = new Cancel(message)
})

@@ -5,7 +5,8 @@ node_modules/debug/src/browser.js(14,41): error TS2304: Cannot find name 'chrome
node_modules/debug/src/browser.js(15,21): error TS2304: Cannot find name 'chrome'.
node_modules/debug/src/browser.js(48,47): error TS2339: Property 'process' does not exist on type 'Window'.
node_modules/debug/src/browser.js(48,65): error TS2339: Property 'process' does not exist on type 'Window'.
node_modules/debug/src/browser.js(59,139): error TS2551: Property 'WebkitAppearance' does not exist on type 'CSSStyleDeclaration'. Did you mean 'webkitAppearance'?
node_modules/debug/src/browser.js(59,99): error TS2339: Property 'style' does not exist on type 'HTMLElement'.
node_modules/debug/src/browser.js(59,133): error TS2339: Property 'style' does not exist on type 'HTMLElement'.
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 is going on here either -- the lib.dom.d.ts didn't change around HTMLElement, and only to add an index signature. debug itself hasn't changed either. And I don't think document.documentElement was previously any by mistake, because the previous type error was on document.documentElement.style.

@@ -100,15 +100,15 @@ node_modules/lodash/_createFlow.js(56,13): error TS2454: Variable 'wrapper' is u
node_modules/lodash/_createFlow.js(57,13): error TS2454: Variable 'wrapper' is used before being assigned.
node_modules/lodash/_createFlow.js(57,21): error TS2339: Property 'thru' does not exist on type 'LodashWrapper'.
node_modules/lodash/_createFlow.js(65,24): error TS2339: Property 'plant' does not exist on type 'LodashWrapper'.
node_modules/lodash/_createHybrid.js(44,49): error TS2345: Argument of type 'string | Function' is not assignable to parameter of type 'Function'.
node_modules/lodash/_createHybrid.js(44,49): error TS2345: Argument of type 'TimerHandler' is not assignable to parameter of type 'Function'.
Copy link
Member

Choose a reason for hiding this comment

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

generated d.ts now has an alias type TimerHandler = string | Function. This, uh, happens a lot in places that don't handle timers. I don't think it should be a built-in alias.

@@ -165,6 +165,7 @@ node_modules/lodash/bind.js(55,6): error TS2339: Property 'placeholder' does not
node_modules/lodash/bindKey.js(62,53): error TS2454: Variable 'holders' is used before being assigned.
node_modules/lodash/bindKey.js(66,9): error TS2339: Property 'placeholder' does not exist on type 'Function'.
node_modules/lodash/castArray.js(10,15): error TS8029: JSDoc '@param' tag has name 'value', but there is no parameter with that name. It would match 'arguments' if it had an array type.
node_modules/lodash/chain.js(33,16): error TS2348: Value of type 'typeof lodash' is not callable. Did you mean to include 'new'?
Copy link
Member

Choose a reason for hiding this comment

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

in wrapperLodash.js, function lodash is marked with @constructor, despite not referencing this. It is literally a wrapper for calling return new LodashWrapper along with a few other cases. It's not a constructor function, but a factory function.

@@ -30,6 +30,8 @@ node_modules/npm/bin/npm-cli.js(132,17): error TS2339: Property 'config' does no
node_modules/npm/bin/npm-cli.js(134,17): error TS2339: Property 'config' does not exist on type 'typeof EventEmitter'.
node_modules/npm/bin/npm-cli.js(136,17): error TS2339: Property 'config' does not exist on type 'typeof EventEmitter'.
node_modules/npm/html/static/toc.js(3,40): error TS2531: Object is possibly 'null'.
node_modules/npm/html/static/toc.js(11,7): error TS2339: Property 'innerHTML' does not exist on type 'HTMLUListElement'.
node_modules/npm/html/static/toc.js(28,3): error TS2531: Object is possibly 'null'.
Copy link
Member

Choose a reason for hiding this comment

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

These two are a result of generated DOM changes too.

@sandersn
Copy link
Member

sandersn commented Aug 1, 2018

This is one of the PRs at fault. microsoft/TypeScript-DOM-lib-generator#524

@weswigham
Copy link
Member

I reran the user tests post-lib changes and #26156 has the diff. The changes look like the expected/acceptable ones to me - @sandersn wanna merge if you agree?

@weswigham weswigham closed this Aug 2, 2018
@sandersn
Copy link
Member

sandersn commented Aug 2, 2018

I'll take a look.

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.

4 participants