Skip to content

Commit

Permalink
fix: don't parse base class when empty constructor is defined
Browse files Browse the repository at this point in the history
BREAKING CHANGE: This breaks consumers that rely
 on the existing behavior where if a constructor was
 defined but with no parameters, the base class was
 parsed. Closes #198.
  • Loading branch information
jeffijoe committed Oct 30, 2021
1 parent 85fdc99 commit bfe2266
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 32 deletions.
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@
"@babel/preset-env",
{
"targets": {
"browsers": "last 2 versions",
"node": "8.0.0"
"node": "12.0.0"
}
}
]
Expand Down
22 changes: 4 additions & 18 deletions src/__tests__/inheritance.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,21 @@ test('parses parent classes if there are no declared parameters', () => {
}
}

class Level2 extends Level1 {
constructor() {
super(...arguments)
}
}
class Level2 extends Level1 {}

class Level3 extends Level2 {}

class Level4 extends Level2 {
constructor(level4Arg1) {
class Level3 extends Level2 {
constructor(level3Arg1) {
super(...arguments)
}
}

container.register({
level1Arg1: asValue(1),
level1Arg2: asValue(2),
level4Arg1: asValue(4),
level3Arg1: asValue(4),
level1: asClass(Level1),
level2: asClass(Level2),
level3: asClass(Level3),
level4: asClass(Level4),
})

expect(container.resolve('level1')).toEqual(
Expand All @@ -53,13 +46,6 @@ test('parses parent classes if there are no declared parameters', () => {
)

expect(container.resolve('level3')).toEqual(
expect.objectContaining({
arg1: 1,
arg2: 2,
})
)

expect(container.resolve('level4')).toEqual(
expect.objectContaining({
arg1: 4,
arg2: undefined,
Expand Down
14 changes: 14 additions & 0 deletions src/__tests__/param-parser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,20 @@ describe('parseParameterList', () => {
])
})

it('returns null when no constructor is defined', () => {
class Test {
dep1: any

method() {
/**/
}
}

expect(parseParameterList(Test.prototype.constructor.toString())).toBe(null)

expect(parseParameterList('class Lol extends Wee {}')).toBe(null)
})

it('supports carriage return in function signature', () => {
expect(parseParameterList(`function (\r\ndep1,\r\ndep2\r\n) {}`)).toEqual([
{ name: 'dep1', optional: false },
Expand Down
12 changes: 9 additions & 3 deletions src/param-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ export interface Parameter {
* @param {string} source
* The source of a function to extract the parameter list from
*
* @return {Array<Parameter>}
* Returns an array of parameters.
* @return {Array<Parameter> | null}
* Returns an array of parameters, or `null` if no
* constructor was found for a class.
*/
export function parseParameterList(source: string): Array<Parameter> {
export function parseParameterList(source: string): Array<Parameter> | null {
const { next: _next, done } = createTokenizer(source)
const params: Array<Parameter> = []

Expand All @@ -33,6 +34,11 @@ export function parseParameterList(source: string): Array<Parameter> {
switch (t.type) {
case 'class':
skipUntilConstructor()
// If we didn't find a constructor token, then we know that there
// are no dependencies in the defined class.
if (!isConstructorToken()) {
return null
}
// Next token is the constructor identifier.
nextToken()
break
Expand Down
20 changes: 11 additions & 9 deletions src/resolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -488,18 +488,20 @@ function generateResolve(fn: Function, dependencyParseTarget?: Function) {

/**
* Parses the dependencies from the given function.
* If it's a class and has an extends clause, and no reported dependencies, attempt to parse it's super constructor.
* If it's a class that extends another class, and it does
* not have a defined constructor, attempt to parse it's super constructor.
*/
function parseDependencies(fn: Function): Array<Parameter> {
const result = parseParameterList(fn.toString())
if (result.length > 0) {
return result
}

const parent = Object.getPrototypeOf(fn)
if (typeof parent === 'function' && parent !== Function.prototype) {
// Try to parse the parent
return parseDependencies(parent)
if (!result) {
// No defined constructor for a class, check if there is a parent
// we can parse.
const parent = Object.getPrototypeOf(fn)
if (typeof parent === 'function' && parent !== Function.prototype) {
// Try to parse the parent
return parseDependencies(parent)
}
return []
}

return result
Expand Down

0 comments on commit bfe2266

Please sign in to comment.