[helpers TS conversion] object #16505
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/56945 |
| var keys: string[] = Object.getOwnPropertyNames(defaults), i = 0; | ||
| i < keys.length; | ||
| i++ |
There was a problem hiding this comment.
If I hoist the keys to the scope of the _defaults, the generated size will be increased by 3 ~ 5 bytes.
| // @ts-expect-error -- intentionally omitting the argument | ||
| Object.assign.bind(/* undefined */) |
There was a problem hiding this comment.
If undefined is explicitly given, the generated size will increase by about 6 bytes. So I omit the undefined and add a ts-expect-error.
| export default function _objectDestructuringEmpty<T>( | ||
| obj: T | null | undefined, | ||
| ): asserts obj is T { | ||
| if (obj == null) throw new TypeError("Cannot destructure " + obj); | ||
| } |
There was a problem hiding this comment.
_objectDestructuringEmpty is basically a nullthrow with a custom error message, so asserts is used.
| /* @onlyBabel7 */ | ||
|
|
||
| // @ts-expect-error Migrate in another PR | ||
| import defineProperty from "defineProperty.ts"; |
There was a problem hiding this comment.
| import defineProperty from "defineProperty.ts"; | |
| import defineProperty from "./defineProperty.ts"; |
There was a problem hiding this comment.
Oh, I missed that part! Fixing now.
| @@ -1,26 +1,47 @@ | |||
| /* @minVersion 7.5.0 */ | |||
|
|
|||
| // @ts-expect-error Migrate in another PR | |||
| import defineProperty from "defineProperty"; | |||
There was a problem hiding this comment.
| import defineProperty from "defineProperty"; | |
| import defineProperty from "./defineProperty.ts"; |
(this need to match the final path, so that TS will stop complaining once defineProperty is migrated and it will tell us to remove the @ts-expect-error)
There was a problem hiding this comment.
And I missed this one as well, fixing.
|
@nicolo-ribaudo @liuxingbaoyu I've applied the code review suggestions in |
| object: object, | ||
| enumerableOnly?: boolean | undefined, | ||
| ): PropertyKey[] { | ||
| var keys: PropertyKey[] = Object.keys(object); |
There was a problem hiding this comment.
This can be narrowed down to (string | symbol)[]. The PropertyKey is supposed to type the valid non-computed key syntax in the object literal, which also contains number, however, Object.keys() will always return string.
There was a problem hiding this comment.
Actually, Object.keys only returns strings and not symbols. TypeScript's build-in definition of Object.keys is fine.
There was a problem hiding this comment.
Oh I see, we later push symbols.
nicolo-ribaudo
changed the title
| @@ -204,7 +204,7 @@ const helpers: Record<string, Helper> = { | |||
| // size: 252, gzip size: 188 | |||
| extends: helper( | |||
| "7.0.0-beta.0", | |||
| "export default function _extends(){return _extends=Object.assign?Object.assign.bind():function(n){for(var e=1;e<arguments.length;e++){var t=arguments[e];for(var r in t)({}).hasOwnProperty.call(t,r)&&(n[r]=t[r])}return n},_extends.apply(this,arguments)}", | |||
| "export default function _extends(){return _extends=Object.assign?Object.assign.bind():function(n){for(var e=1;e<arguments.length;e++){var t=arguments[e];for(var r in t)({}).hasOwnProperty.call(t,r)&&(n[r]=t[r])}return n},_extends.apply(null,arguments)}", | |||
There was a problem hiding this comment.
I changed this to null so that we don't have to add a this: any parameter to the function, which doesn't actually use this.
|
Thanks, and sorry for the many extra commits :) |
Migrate all Object related Babel helpers (
extends,defaults,objectWithoutProperties,objectSpread,objectSpread2) to TypeScript with reasonable types (and a little TypeScript exercise~). The PR is made in a way that preventsyarn gulp generate-runtime-helpersfrom introducing changes.