Method decorators break with compilerOptions specified

[mrk3767]

Behaviour seems to be different when compilerOptions are included vs not included for decorators. This causes method decorators not to function as expected.

TypeScript Version: 3.2.0-dev.201xxxxx

#!/usr/bin/env node
var ts = require("typescript")
var fs = require("fs")
const shell = require('shelljs')

const original = `
    function logMethod(target, key, descriptor) {
    
        let originalMethod: () => any
        if (descriptor === undefined) {
            descriptor = Object.getOwnPropertyDescriptor(target, key)
        }
        originalMethod = descriptor.value
    
        // editing the descriptor/value parameter
        descriptor.value = function(...args: any[]) {
            console.log('Decorated method ' + key + ' logged with args:[' + args + '}]')
            const result = originalMethod.apply(this, args)
            return result
        }
    
        //Object.defineProperty(target, key, descriptor)
        // return edited descriptor as opposed to overwriting the descriptor
        return descriptor
    }
    
    export class Example {
        @logMethod
        public calc(x: number, y: number): number {
            return x + y;
        }
    }
    
    var main = function () {
        new Example().calc(5, 4);
        console.log("Finished running example");
    }()
`

const fail = (source) => {
    return ts.transpileModule(source, {compilerOptions: {}}).outputText
}

const pass = (s) => {
    return ts.transpileModule(s, {}).outputText
}

(async function () {
    let transpiledWrong, transpiledRight
    await new Promise((resolve, reject) => {
        let filesWritten = 0
        let checkFile = (err) => {
            if( ++filesWritten >= 2 )
                err ? console.log(err) : console.log("file written")
                resolve()
        }

        fs.writeFile('fail.js', transpiledWrong = fail(original), (err) => {
            checkFile(err)
        })
        fs.writeFile('pass.js', transpiledRight = pass(original), (err) => {
            checkFile(err)
        })

    })

    console.log(`
    When transpiled with an empty object passed in for the config, a null is added to the __descriptor() call below

===== Begin file out =====

${transpiledRight}
===== End file out =====

    The expected output is
        
        Decorated method calc logged with args:[5,4}]
        Finished running example
          
    The actual output follows  

${shell.exec('node ./pass.js')}

    When transpiled with an object containing compilerOptions (with or without options) is passed in for the config, a null is missing from the __descriptor() call below

===== Begin file out =====

${transpiledWrong}
===== End file out =====

    The expected output is
        
        Decorated method calc logged with args:[5,4}]
        Finished running example
          
    The actual output follows  

${shell.exec('node ./fail.js')}
    `)
})()

[weswigham]

Providing a compilerOptions object overrides the defaults for transpileModule. The defaults imply target: es5 and jsx: preserve (while explicitly providing an empty object makes the target es3 and no jsx allowed). The difference in target is probably what causes the discrepancy.

[mrk3767]

Thanks for the quick response. I believe this implies that method decorators are unsupported in es3? Would you happen to know why that is the case?

I have been able to make them work by manually adding a null parameter to the __decorator() call in the js output.

[weswigham]

I'm pretty sure it's because property descriptors aren't a thing in es3; but almost nobody uses es3 anymore~

[DanielRosenwasser]

Seems like our defaults should still be filled in.

[mrk3767]

@weswigham - Google Apps Script is still all es3 haha

Some things I played around with

Modifying the __decorator function generation code

return c > 3 && r && Object.defineProperty(target, key, r), r;
to
return c > 3 && r && Object.defineProperty(target, key, r), r;

https://github.com/Microsoft/TypeScript/blob/c2898db9dd58c9b0681d6131ed6076d71d89e6b5/src/compiler/transformers/ts.ts#L3686

Result: Works for my case, breaks all the tests

Modifying the __decorator call generation code

https://github.com/Microsoft/TypeScript/blob/c2898db9dd58c9b0681d6131ed6076d71d89e6b5/src/compiler/transformers/ts.ts#L1571

Result: Couldn't seem to get any different behavior changing any of this, but it looks like it's where the problem would be

[weswigham]

If you need to, you can always just supply your own __decorator helper that works sufficiently well on your runtime.

[rbuckton]

You would also likely see a diagnostic reported to the effect that ES3 is not supported if you were to have written this instead:

const fail = (source) => {
    const result = ts.transpileModule(source, { reportDiagnostics: true, compilerOptions: {}});
    if (result.diagnostics.length) {
        console.log(result.diagnostics); // related "option" diagnostic will be here when when reportDiagnostics is true
    }
    return result.outputText;
}

[rbuckton]

@DanielRosenwasser what is the action item here? Should we mix in the defaults for transpileModule, or just use the defaults if the compilerOptions property is an empty object?

[DanielRosenwasser]

I believe mix in the defaults for transpileModule and override whatever the user gives us.