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 for refs/heads/master #38128

Merged
merged 1 commit into from
May 7, 2020

Conversation

typescript-bot
Copy link
Collaborator

This test run was triggerd by a request on #33716
Please review the diff and merge if no changes are unexpected.
You can view the build log here.

cc @microsoft/typescript

@typescript-bot typescript-bot force-pushed the user-baseline-updates branch 29 times, most recently from fad880a to 0d34fc2 Compare April 28, 2020 18:48
@typescript-bot typescript-bot force-pushed the user-baseline-updates branch 18 times, most recently from 9a4016c to 612071e Compare May 6, 2020 23:04
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.

I still need to look at a couple of projects, uglify and webpack.

  • I found one bug.
  • I'm not sure whether pop changed, but it's the only reason I can think of for the new errors in enhanced-resolve.
  • Everything else I looked at seems correct.

@fluentui/ability-attributes: internal/modules/cjs/loader.js:491
@fluentui/ability-attributes: throw new ERR_PACKAGE_PATH_NOT_EXPORTED(basePath, mappingKey);
@fluentui/ability-attributes: ^
@fluentui/ability-attributes: Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: No "exports" main resolved in /office-ui-fabric-react/node_modules/@babel/helper-compilation-targets/package.json
Copy link
Member

Choose a reason for hiding this comment

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

@weswigham think this is our fault?

at throwPluginError (/vue-next/node_modules/rollup/dist/shared/rollup.js:16838:12)
at Object.error (/vue-next/node_modules/rollup/dist/shared/rollup.js:17857:24)
at Object.error (/vue-next/node_modules/rollup/dist/shared/rollup.js:17011:38)
[!] (plugin rpt2) Error: /vue-next/packages/runtime-core/src/apiInject.ts(40,9): semantic error TS2360: The left-hand side of an 'in' expression must be of type 'any', 'string', 'number', or 'symbol'.
Copy link
Member

Choose a reason for hiding this comment

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

I think this is new, and should be checked for correctness.

Edit: Yes, seems correct, and from the new union-in change.

@@ -91,6 +91,7 @@ node_modules/adonis-framework/src/Helpers/index.js(256,45): error TS2345: Argume
Type 'undefined' is not assignable to type 'string'.
node_modules/adonis-framework/src/Helpers/index.js(330,23): error TS2532: Object is possibly 'undefined'.
node_modules/adonis-framework/src/Middleware/index.js(13,21): error TS2307: Cannot find module 'adonis-fold' or its corresponding type declarations.
node_modules/adonis-framework/src/Middleware/index.js(87,38): error TS1016: A required parameter cannot follow an optional parameter.
Copy link
Member

Choose a reason for hiding this comment

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

we're now checking JSDoc for this, and is expected
and is OK as long as it doesn't happen too much

@@ -275,6 +275,9 @@ node_modules/acorn/dist/acorn.js(5299,5): error TS2339: Property 'nextToken' doe
node_modules/acorn/dist/acorn.js(5300,12): error TS2339: Property 'parseExpression' does not exist on type 'Parser'.
node_modules/acorn/dist/acorn.js(5307,10): error TS2554: Expected 3 arguments, but got 2.
node_modules/acorn/dist/acorn_loose.es.js(45,12): error TS2554: Expected 3 arguments, but got 2.
node_modules/acorn/dist/acorn_loose.es.js(64,7): error TS2532: Object is possibly 'undefined'.
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 from Anders' new control flow checking of this-property assignments in the constructor -- acorn's Node only initialises about half of its properties under certain config settings.

@@ -275,6 +275,9 @@ node_modules/acorn/dist/acorn.js(5299,5): error TS2339: Property 'nextToken' doe
node_modules/acorn/dist/acorn.js(5300,12): error TS2339: Property 'parseExpression' does not exist on type 'Parser'.
node_modules/acorn/dist/acorn.js(5307,10): error TS2554: Expected 3 arguments, but got 2.
node_modules/acorn/dist/acorn_loose.es.js(45,12): error TS2554: Expected 3 arguments, but got 2.
node_modules/acorn/dist/acorn_loose.es.js(64,7): error TS2532: Object is possibly 'undefined'.
node_modules/acorn/dist/acorn_loose.es.js(64,23): error TS2532: Object is possibly 'undefined'.
node_modules/acorn/dist/acorn_loose.es.js(66,7): error TS2532: Object is possibly 'undefined'.
Copy link
Member

Choose a reason for hiding this comment

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

there's no error though -- the editor isn't associating files with the parent tsconfig anymore.

@@ -39,8 +42,6 @@ node_modules/enhanced-resolve/lib/Resolver.js(211,13): error TS2339: Property 'm
node_modules/enhanced-resolve/lib/Resolver.js(262,20): error TS2339: Property 'recursion' does not exist on type 'Error'.
node_modules/enhanced-resolve/lib/concord.js(80,30): error TS2531: Object is possibly 'null'.
node_modules/enhanced-resolve/lib/concord.js(81,17): error TS2531: Object is possibly 'null'.
node_modules/enhanced-resolve/lib/createInnerCallback.js(22,20): error TS2339: Property 'stack' does not exist on type '(...args: any[]) => any'.
Copy link
Member

Choose a reason for hiding this comment

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

fix from non-top-level property assignments

@@ -1,5 +1,10 @@
Exit Code: 1
Standard output:
node_modules/enhanced-resolve/lib/CachedInputFileSystem.js(120,20): error TS2532: Object is possibly 'undefined'.
Copy link
Member

Choose a reason for hiding this comment

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

I guess.. pop now returns T | undefined instead of just T?

Copy link
Member

Choose a reason for hiding this comment

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

nope, previously this.levels: any[], now it's this.levels: Set<any>[], so pop now returns Set<any> | undefined not any | undefined ==> any.

@@ -5,28 +5,40 @@ node_modules/lodash/_ListCache.js(20,17): error TS2532: Object is possibly 'unde
node_modules/lodash/_MapCache.js(20,17): error TS2532: Object is possibly 'undefined'.
node_modules/lodash/_SetCache.js(19,14): error TS2532: Object is possibly 'undefined'.
node_modules/lodash/_Stack.js(17,20): error TS2339: Property 'size' does not exist on type 'ListCache'.
node_modules/lodash/_arrayAggregator.js(11,33): error TS1016: A required parameter cannot follow an optional parameter.
Copy link
Member

Choose a reason for hiding this comment

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

lodash consistently uses brackets to mean that the type includes null | undefined, not that it's possibly missing. eg

/** @param {Array} [array] The array to iterate over */

would be written in TS as

array: Array<unknown> | null | undefined

@@ -83,6 +83,7 @@ node_modules/npm/lib/cache.js(16,34): error TS2339: Property 'commands' does not
node_modules/npm/lib/cache.js(49,30): error TS2339: Property 'prefix' does not exist on type 'typeof EventEmitter'.
node_modules/npm/lib/cache.js(69,35): error TS2339: Property 'cache' does not exist on type 'typeof EventEmitter'.
node_modules/npm/lib/cache.js(70,12): error TS2339: Property 'config' does not exist on type 'typeof EventEmitter'.
node_modules/npm/lib/cache.js(82,3): error TS2775: Assertions require every name in the call target to be declared with an explicit type annotation.
Copy link
Member

Choose a reason for hiding this comment

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

haha wow this is going to be super annoying for anybody who turns on checkJS and then uses assert from @types/node. Hope this is only with strictNullChecks on.

Copy link
Member

Choose a reason for hiding this comment

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

later: no this is a bug when importing assertions via commonjs.

Filed #38379 to track it.

@sandersn
Copy link
Member

sandersn commented May 6, 2020

@sheetalkamat any idea why opening .js files inside a node_modules might not find a parent tsconfig anymore? The includes pattern of these user tests has node_modules/npm (for example).

@typescript-bot typescript-bot force-pushed the user-baseline-updates branch 2 times, most recently from c76d9a9 to a7d86de Compare May 7, 2020 16:12
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.

I'll merge this after I decide whether to file a bug on our interpretation of postfix-= as optional

lib/optimize/ConcatenatedModule.js(205,32): error TS2554: Expected 1 arguments, but got 0.
lib/util/fs.js(57,23): error TS1016: A required parameter cannot follow an optional parameter.
lib/util/fs.js(78,19): error TS1016: A required parameter cannot follow an optional parameter.
lib/util/fs.js(98,22): error TS1016: A required parameter cannot follow an optional parameter.
Copy link
Member

Choose a reason for hiding this comment

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

Here, webpack uses {T=} to indicate T | undefined, not possibly-missing. I thought that's what we and closure understood it to mean, too, so I need to go check.

Copy link
Member

Choose a reason for hiding this comment

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

Nope, I forgot. ?T is the syntax for T | null, T= is the syntax for optional. Closure doesn't distinguish between null and undefined iirc.

@@ -1,9 +1,22 @@
Exit Code: 1
Standard output:
lib/AbstractMethodError.js(26,16): error TS2532: Object is possibly 'undefined'.
Copy link
Member

Choose a reason for hiding this comment

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

The compiler doesn't know that Error.captureStackThis(this) will set the property this.stack. Seems fair tbh.

@@ -1,9 +1,22 @@
Exit Code: 1
Standard output:
lib/AbstractMethodError.js(26,16): error TS2532: Object is possibly 'undefined'.
lib/ContextModule.js(333,29): error TS2345: Argument of type 'Error' is not assignable to parameter of type 'WebpackError'.
Copy link
Member

Choose a reason for hiding this comment

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

better type from control-flow tracking in constructor

@sandersn sandersn merged commit f08863d into microsoft:master May 7, 2020
@sheetalkamat
Copy link
Member

@sandersn #35011 changed that behaviour

@sandersn
Copy link
Member

sandersn commented May 7, 2020

Thanks for the link. So far, it looks like a workaround is to start in index.ts outside node_modules and navigate into node_modules to edit the package that is being tested.

cangSDARM added a commit to cangSDARM/TypeScript that referenced this pull request May 12, 2020
* upstream/master: (54 commits)
  LEGO: check in for master to temporary branch.
  LEGO: check in for master to temporary branch.
  Fix for jsdoc modifiers on constructor params (microsoft#38403)
  Improve assert message in binder (microsoft#38270)
  fix broken regex on "src/services/completions.ts#getCompletionData" (microsoft#37546)
  report error for duplicate @type declaration (microsoft#38340)
  fix(38073): hide 'Extract to function in global scope' action for arrow functions which use 'this' (microsoft#38107)
  Update user baselines (microsoft#38472)
  Update user baselines (microsoft#38405)
  Changed template strings to emit void 0 instead of undefined (microsoft#38430)
  Fix js missing type arguments on existing nodes and jsdoc object literal declaration emit (microsoft#38368)
  LEGO: check in for master to temporary branch.
  Make isDynamicFileName available publicly (microsoft#38269)
  LEGO: check in for master to temporary branch.
  LEGO: check in for master to temporary branch.
  Exclude arrays and tuples from full intersection property check (microsoft#38395)
  Fix crash caused by assertion with evolving array type (microsoft#38398)
  Update user baselines (microsoft#38128)
  LEGO: check in for master to temporary branch.
  moveToNewFile: handle namespace imports too
  ...

# Conflicts:
#	src/compiler/types.ts
#	src/compiler/utilities.ts
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