Typescript: fields defined with definite assignment assertion are removed with TS compiler flag useDefineForClassFields set

[akphi]

Bug Report

Current behavior
If I have a field defined using !, for example:

class Person {
  job?: number;
  age!: number;

  constructor() {
    
  }
}

babel using preset-typescript will transform it to this (REPL link):

class Person {
  constructor() {}
}

I think is the behavior currently being implemented, given this test case.

This is the same behavior I observe in Typescript version < 3.7, see this playground link

But Typescript 3.7 comes with the new compiler flag useDefineForClassFields (doc). With this option enabled, see this playground link, for the same input above, what Typescript generates is:

"use strict";
class Person {
    job;
    age;
    constructor() {
    }
}

Expected behavior
I think I get the decision behind stripping out fields defined using definite assignment assertion, so conceptually, they are no different than fields defined using declare. However, for my use case, I would really like babel to be in sycn with Typescript here for the case where useDefineForClassFields is set to true, that is I want to fields to be present and initialized to undefined. I think I have seen exception made in the past for not stripping away ! fields like in #8238. I wonder if we can add a new setting to preset-typescript or make another exception.

Environment
I thought the above REPLs and playgrounds should suffice, but in case you need a minimal repo, I prepared this repo. For this the important bits in my configs are:

// tsconfig.json
{
  ...
  "useDefineForClassFields": true,
  ...
}

// babel settings (in webpack)

presets: [
      '@babel/preset-env',
      '@babel/preset-react',
      './dev/test-preset',
      ['@babel/preset-typescript', {
        onlyRemoveTypeImports: true,
        allowDeclareFields: true
      }],
    ]

// where `dev/test-preset` looks like the following 

plugins: [
      ['@babel/plugin-proposal-decorators', { legacy: true }],
      ['@babel/plugin-proposal-class-properties', { loose: true }],
    ],

Additional context
If this is appropriate, I want to mention my use case. I have some classes where the constructor is not a good place to initialize some of the fields, but I want to keep defining them using ! because they should not be nullable (If only JS/TS support overloading constructors, this would be a non-problem). I'm also using mobx, which creates an observable wrapper around fields so the recommended place to set this is in the constructor, and mobx cannot really wrap fields that do not exist because babel strips away the field defined with ! during build time. mobx used to work fine in the past because they use the decorator form, i.e. @observable age!: number which at the time worked because of #8238

Also, another totally coincidental thing is I used to have @babel/plugin-proposal-class-properties running before preset-typescript so it emits ! fields and things work just fine, but recently, because I use declare field like declare name: string, I have to make sure preset-typescript runs before @babel/plugin-proposal-class-properties. So this problem surfaces, then on that topic, there's #10311

Anyhow, this has been a long report, please let me know what you think or what I can (maybe) help with since I'm still pretty inexperience. Thanks a lot!

[babel-bot]

Hey @akphi! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite."

[nicolo-ribaudo]

It seems like our logic in the plugin is wrong:

babel/packages/babel-plugin-transform-typescript/src/index.js

Lines 72 to 90 in 3ec4537

	if (node.definite || node.declare) {
	if (node.value) {
	throw path.buildCodeFrameError(
	`Definitely assigned fields and fields with the 'declare' modifier cannot` +
	` be initialized here, but only in the constructor`,
	);
	}
	if (!node.decorators) {
	path.remove();
	}
	} else if (
	!allowDeclareFields &&
	!node.value &&
	!node.decorators &&
	!t.isClassPrivateProperty(node)
	) {
	path.remove();
	}

We unified the error handling for initializers in declare and definite fields, and accidentally also unified the logic to remove the class field. Are you interested in opening a PR to fix it? (I'll label this as "good first issue" otherwise").

[nicolo-ribaudo]

Please note that we should keep the field only when allowDeclareFields (the equivalent of useDefineForClassFields) is true.

[akphi]

@nicolo-ribaudo Thanks for giving the opportunity. I will give it a shot!

[nicolo-ribaudo]

In case you need any help with getting started (feel free to ignore this message otherwise):

---

If it is the first time that you contribute to Babel, follow these steps: (you need to have make and yarn available on your machine)

1. Write a comment there to let other possible contributors know that you are working on this bug. (done ✔️)
2. Fork the repo
3. Run git clone https://github.com/<YOUR_USERNAME>/babel.git && cd babel
4. Run yarn && make bootstrap
5. Wait ⏳
6. Run make watch (or make build whenever you change a file)
7. Add a test (only input.ts; output.js will be automatically generated)
8. Update the code!
9. yarn jest typescript to run the tests
   • If some test outputs don't match but the new results are correct, you can delete the bad output.js files and run the tests again
   • If you prefer, you can run OVERWRITE=true yarn jest typescript and they will be automatically updated.
10. If it is working, run make test to run all the tests
11. Run git push and open a PR!

[akphi]

@nicolo-ribaudo that was super helpful :) I created an MR, please could you take a look? Also, if this is merged, could we do a release for @babel/transform-typescript anytime soon?