Extract source map generation logic out of the emitter. #5780
Conversation
| @@ -2414,6 +2414,27 @@ namespace ts { | |||
| return output; | |||
| } | |||
|
|
|||
| export const stringify: (value: any) => string = JSON && JSON.stringify ? JSON.stringify : function stringify(value: any): string { | |||
| /* tslint:disable:no-null */ | |||
| return value == null ? "null" | |||
There was a problem hiding this comment.
Should use === when comparing with null - as written this lumps undefined with null. If value is undefined, JSON.stringify should return undefined. A key with an undefined value should be omitted, but an array with undefined slots should have those slots serialized as null (as this does). Presently, a map with undefined valued keys gets encoded with null values, since Object.prototype.hasOwnProperty.call({x: undefined}, 'x') returns true.
There was a problem hiding this comment.
Wait, no, nevermind, stringifyProperty has a guard for undefined. That was confusing.
There was a problem hiding this comment.
Actually it should be value === null, as that's how I have it in the experimentalTransforms branch. This was a typo introduced while trying to work out how to disable a tslint rule :)
There was a problem hiding this comment.
undefined does get stringified to "null" as expected by the last case, too. :3
|
👍 |
| export function getNullSourceMapWriter(): SourceMapWriter { | ||
| if (nullSourceMapWriter === undefined) { | ||
| nullSourceMapWriter = { | ||
| getSourceMapData: nop, |
There was a problem hiding this comment.
this needs to be checked, but AFAIR calling vararg function with different number of arguments is not a very good pattern for V8. When source map code was written initially it has the same shape but then it was rewritten so each nop function has the same number of arguments with the corresponding method in the interface.
There was a problem hiding this comment.
@vladima is right. We had seen perf impact of not using same number of arguments in the no-op functions
There was a problem hiding this comment.
I can add stubs for these, though when I ran the performance tests the cost looked negligible (and the performance tests don't include source maps, so they would be hitting this case). I do wonder if v8 has any optimizations for the Function.prototype primordial. If I see a major difference I'll push up a new commit.
There was a problem hiding this comment.
There does seem to be a small improvement, so I'm adding the stubs.
|
👍 |
|
👍 |
Extract source map generation logic out of the emitter.
Supports #5595.