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
fix: push newClass
only when class is decorated
#14334
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -491,13 +491,8 @@ function applyMemberDecs( | |
); | ||
} | ||
|
||
if (protoInitializers.length > 0) { | ||
pushInitializers(ret, protoInitializers); | ||
} | ||
|
||
if (staticInitializers.length > 0) { | ||
pushInitializers(ret, staticInitializers); | ||
} | ||
pushInitializers(ret, protoInitializers); | ||
pushInitializers(ret, staticInitializers); | ||
} | ||
|
||
function pushInitializers(ret, initializers) { | ||
|
@@ -521,23 +516,25 @@ function pushInitializers(ret, initializers) { | |
|
||
function applyClassDecs(ret, targetClass, metadataMap, classDecs) { | ||
var initializers = []; | ||
var newClass = targetClass; | ||
|
||
var name = targetClass.name; | ||
var ctx = Object.assign( | ||
{ | ||
kind: "class", | ||
name: name, | ||
addInitializer: createAddInitializerMethod(initializers), | ||
}, | ||
createMetadataMethodsForProperty(metadataMap, 0 /* CONSTRUCTOR */, name) | ||
); | ||
if (classDecs.length > 0) { | ||
var newClass = targetClass; | ||
|
||
var name = targetClass.name; | ||
var ctx = Object.assign( | ||
{ | ||
kind: "class", | ||
name: name, | ||
addInitializer: createAddInitializerMethod(initializers), | ||
}, | ||
createMetadataMethodsForProperty(metadataMap, 0 /* CONSTRUCTOR */, name) | ||
); | ||
|
||
for (var i = classDecs.length - 1; i >= 0; i--) { | ||
newClass = classDecs[i](newClass, ctx) || newClass; | ||
} | ||
for (var i = classDecs.length - 1; i >= 0; i--) { | ||
newClass = classDecs[i](newClass, ctx) || newClass; | ||
} | ||
|
||
ret.push(newClass); | ||
ret.push(newClass); | ||
} | ||
|
||
if (initializers.length > 0) { | ||
ret.push(function () { | ||
|
@@ -699,23 +696,19 @@ export default function applyDecs(targetClass, memberDecs, classDecs) { | |
var ret = []; | ||
var staticMetadataMap = {}; | ||
|
||
if (memberDecs) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The guard is removed because the transformer always provides an array literal for |
||
var protoMetadataMap = {}; | ||
var protoMetadataMap = {}; | ||
|
||
applyMemberDecs( | ||
ret, | ||
targetClass, | ||
protoMetadataMap, | ||
staticMetadataMap, | ||
memberDecs | ||
); | ||
applyMemberDecs( | ||
ret, | ||
targetClass, | ||
protoMetadataMap, | ||
staticMetadataMap, | ||
memberDecs | ||
); | ||
|
||
convertMetadataMapToFinal(targetClass.prototype, protoMetadataMap); | ||
} | ||
convertMetadataMapToFinal(targetClass.prototype, protoMetadataMap); | ||
|
||
if (classDecs) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same for |
||
applyClassDecs(ret, targetClass, staticMetadataMap, classDecs); | ||
} | ||
applyClassDecs(ret, targetClass, staticMetadataMap, classDecs); | ||
|
||
convertMetadataMapToFinal(targetClass, staticMetadataMap); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
function dec(fn, context) { | ||
context.addInitializer((instance) => { | ||
instance[context.name + 'Context'] = context; | ||
}); | ||
|
||
return function () { | ||
return fn.call(this) + 1; | ||
} | ||
} | ||
|
||
class Foo { | ||
value = 1; | ||
|
||
@dec | ||
#a() { | ||
return this.value; | ||
} | ||
|
||
callA() { | ||
return this.#a(); | ||
} | ||
} | ||
|
||
let foo = new Foo(); | ||
|
||
const aContext = foo['#aContext']; | ||
|
||
// First call gets the method, second call calls the method with correct `this` | ||
expect(aContext.access.get.call(foo).call(foo)).toBe(2); | ||
expect(foo.callA()).toBe(2); | ||
foo.value = 123; | ||
expect(aContext.access.get.call(foo).call(foo)).toBe(124); | ||
expect(foo.callA()).toBe(124); | ||
|
||
expect(aContext.name).toBe('#a'); | ||
expect(aContext.kind).toBe('method'); | ||
expect(aContext.isStatic).toBe(false); | ||
expect(aContext.isPrivate).toBe(true); | ||
expect(typeof aContext.addInitializer).toBe('function'); | ||
expect(typeof aContext.setMetadata).toBe('function'); | ||
expect(typeof aContext.getMetadata).toBe('function'); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,4 @@ | ||
function dec(fn, context) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The original test case is renamed to |
||
context.addInitializer((instance) => { | ||
instance[context.name + 'Context'] = context; | ||
}); | ||
|
||
return function () { | ||
return fn.call(this) + 1; | ||
} | ||
|
@@ -23,19 +19,6 @@ class Foo { | |
|
||
let foo = new Foo(); | ||
|
||
const aContext = foo['#aContext']; | ||
|
||
// First call gets the method, second call calls the method with correct `this` | ||
expect(aContext.access.get.call(foo).call(foo)).toBe(2); | ||
expect(foo.callA()).toBe(2); | ||
foo.value = 123; | ||
expect(aContext.access.get.call(foo).call(foo)).toBe(124); | ||
expect(foo.callA()).toBe(124); | ||
|
||
expect(aContext.name).toBe('#a'); | ||
expect(aContext.kind).toBe('method'); | ||
expect(aContext.isStatic).toBe(false); | ||
expect(aContext.isPrivate).toBe(true); | ||
expect(typeof aContext.addInitializer).toBe('function'); | ||
expect(typeof aContext.setMetadata).toBe('function'); | ||
expect(typeof aContext.getMetadata).toBe('function'); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
function dec(fn, context) { | ||
context.addInitializer((instance) => { | ||
instance[context.name + 'Context'] = context; | ||
}); | ||
|
||
return function () { | ||
return fn.call(this) + 1; | ||
} | ||
} | ||
|
||
class Foo { | ||
value = 1; | ||
|
||
@dec | ||
a() { | ||
return this.value; | ||
} | ||
|
||
@dec | ||
['b']() { | ||
return this.value; | ||
} | ||
} | ||
|
||
let foo = new Foo(); | ||
|
||
const aContext = foo['aContext']; | ||
const bContext = foo['bContext']; | ||
|
||
expect(foo.a()).toBe(2); | ||
expect(foo.b()).toBe(2); | ||
foo.value = 123; | ||
expect(foo.a()).toBe(124); | ||
expect(foo.b()).toBe(124); | ||
|
||
expect(aContext.name).toBe('a'); | ||
expect(aContext.kind).toBe('method'); | ||
expect(aContext.isStatic).toBe(false); | ||
expect(aContext.isPrivate).toBe(false); | ||
expect(typeof aContext.addInitializer).toBe('function'); | ||
expect(typeof aContext.setMetadata).toBe('function'); | ||
expect(typeof aContext.getMetadata).toBe('function'); | ||
|
||
expect(bContext.name).toBe('b'); | ||
expect(bContext.kind).toBe('method'); | ||
expect(bContext.isStatic).toBe(false); | ||
expect(bContext.isPrivate).toBe(false); | ||
expect(typeof bContext.addInitializer).toBe('function'); | ||
expect(typeof bContext.setMetadata).toBe('function'); | ||
expect(typeof bContext.getMetadata).toBe('function'); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
function dec(fn, context) { | ||
context.addInitializer((instance) => { | ||
instance[context.name + 'Context'] = context; | ||
}); | ||
|
||
return function () { | ||
return fn.call(this) + 1; | ||
} | ||
} | ||
|
||
class Foo { | ||
static value = 1; | ||
|
||
@dec | ||
static #a() { | ||
return this.value; | ||
} | ||
|
||
static callA() { | ||
return this.#a(); | ||
} | ||
} | ||
|
||
const aContext = Foo['#aContext']; | ||
|
||
// First call gets the method, second call calls the method with correct `this` | ||
expect(aContext.access.get.call(Foo).call(Foo)).toBe(2); | ||
expect(Foo.callA()).toBe(2); | ||
Foo.value = 123; | ||
expect(aContext.access.get.call(Foo).call(Foo)).toBe(124); | ||
expect(Foo.callA()).toBe(124); | ||
|
||
expect(aContext.name).toBe('#a'); | ||
expect(aContext.kind).toBe('method'); | ||
expect(aContext.isStatic).toBe(true); | ||
expect(aContext.isPrivate).toBe(true); | ||
expect(typeof aContext.addInitializer).toBe('function'); | ||
expect(typeof aContext.setMetadata).toBe('function'); | ||
expect(typeof aContext.getMetadata).toBe('function'); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
function dec(fn, context) { | ||
context.addInitializer((instance) => { | ||
instance[context.name + 'Context'] = context; | ||
}); | ||
|
||
return function () { | ||
return fn.call(this) + 1; | ||
} | ||
} | ||
|
||
class Foo { | ||
static value = 1; | ||
|
||
@dec | ||
static a() { | ||
return this.value; | ||
} | ||
|
||
@dec | ||
static ['b']() { | ||
return this.value; | ||
} | ||
} | ||
|
||
const aContext = Foo['aContext']; | ||
const bContext = Foo['bContext']; | ||
|
||
expect(Foo.a()).toBe(2); | ||
expect(Foo.b()).toBe(2); | ||
Foo.value = 123; | ||
expect(Foo.a()).toBe(124); | ||
expect(Foo.b()).toBe(124); | ||
|
||
expect(aContext.name).toBe('a'); | ||
expect(aContext.kind).toBe('method'); | ||
expect(aContext.isStatic).toBe(true); | ||
expect(aContext.isPrivate).toBe(false); | ||
expect(typeof aContext.addInitializer).toBe('function'); | ||
expect(typeof aContext.setMetadata).toBe('function'); | ||
expect(typeof aContext.getMetadata).toBe('function'); | ||
|
||
expect(bContext.name).toBe('b'); | ||
expect(bContext.kind).toBe('method'); | ||
expect(bContext.isStatic).toBe(true); | ||
expect(bContext.isPrivate).toBe(false); | ||
expect(typeof bContext.addInitializer).toBe('function'); | ||
expect(typeof bContext.setMetadata).toBe('function'); | ||
expect(typeof bContext.getMetadata).toBe('function'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A dub initializer should be pushed when the member decorators do not introduce initializers, which can only be known at runtime.