alangpierce/sucrase

`disableESTransform` has unexpected side effects

aduh95 opened this issue · 3 comments

import { transform } from "sucrase";

const tsCode = `abstract class Test {
    abstract readonly result: string
  }
`;

console.log({
  withESTransform: transform(tsCode, {
    transforms: ["typescript"],
    disableESTransforms: false,
  }).code,

  withoutESTransform: transform(tsCode, {
    transforms: ["typescript"],
    disableESTransforms: true,
  }).code,
});

Result:

{
  withESTransform: ' class Test {\n    \n  }\n',
  withoutESTransform: ' class Test {\n      result\n  }\n'
}

TypeScript has the same output as the withESTransform one, and that's the one I expected: https://www.typescriptlang.org/play?#code/IYIwzgLgTsDGEAJYBthjAgKgU0gg3gLABQCCokM8CU2wAJgPYB2yAnjbgK7IQBcCSgEtmAcxIBfEkA

For context, I'm using disableESTransforms because I want to keep the code as close as possible to the source.

It matters to me because my code looks like this:

abstract class Parent {
  abstract readonly result: string
}
class Child extends Parent {
  get result() { return 'expected' }
}
console.log(new Child().result) // undefined instead of 'expected'

Hi @aduh95 , sorry for the trouble! This is a known difference in behavior between the original TypeScript implementation of class fields and the now-standard JS class field behavior. You can read about it here: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html#the-usedefineforclassfields-flag-and-the-declare-property-modifier

With disableESTransforms, Sucrase switches to the standard JS behavior, which unfortunately causes issues in your case. I'd recommend:

  • Adding declare to the class field: declare abstract readonly result: string. This tells both TypeScript and Sucrase that Parent.result is just a type declaration and shouldn't produce any code.
  • Enabling useDefineForClassFields in your tsconfig, which changes TypeScript to the JS standard behavior and I believe adds additional checking to help detect issues like the ones you ran into.

In terms of documentation in Sucrase, the README already mentions this issue (though I understand it can be easy to miss), so I don't think there's much more I can do to document this issue, but let me know if you disagree. I'll close this issue since I've answered the question, but feel free to re-open if you have more questions or concerns.

Sorry, upon further testing, it looks like this actually is a difference in behavior between Sucrase and TypeScript, so I'll track this as a bug. It looks like TypeScript treats abstract fields as implicitly declare, so Sucrase should do the same thing.

Thanks for reporting! I just released a fix as Sucrase 3.25.0, so abstract fields will be removed now.