Skip to content
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

Default target esnext is not applied to TypeScript #2012

Closed
LeonEck opened this issue Feb 13, 2022 · 7 comments
Closed

Default target esnext is not applied to TypeScript #2012

LeonEck opened this issue Feb 13, 2022 · 7 comments

Comments

@LeonEck
Copy link

LeonEck commented Feb 13, 2022

I have the following TypeScript code saved in a file called index.ts:

class MyClass {
  private _propertyOne = 1;
  private _propertyTwo = 2;

  printProperties() {
    console.log(this._propertyOne, this._propertyTwo);
  }
}

new MyClass().printProperties();

If I run esbuild without any options, the documentation says that the default target is esnext. But that doesn't seem to be the case at least for TypeScript since I get two different outputs whether I specify the target or not.

Without target:
npx esbuild index.ts

class MyClass {
  constructor() {
    this._propertyOne = 1;
    this._propertyTwo = 2;
  }
  printProperties() {
    console.log(this._propertyOne, this._propertyTwo);
  }
}
new MyClass().printProperties();

With target:
npx esbuild index.ts --target=esnext

class MyClass {
  _propertyOne = 1;
  _propertyTwo = 2;
  printProperties() {
    console.log(this._propertyOne, this._propertyTwo);
  }
}
new MyClass().printProperties();

Here is some additional information.

  • The output of verbose logging to make sure that no tsconfig file is used:
  Read 19 entries for directory "/"
  Read 4 entries for directory "/private"
  Read 32 entries for directory "/private/var"
  Read 3 entries for directory "/private/var/tmp"
  No "browser" map found in directory "/private/var/tmp"
  Attempting to load "/private/var/tmp/index.ts" as a file
    Checking for file "index.ts"
    Found file "index.ts"
  Read 3 entries for directory "/private/var/tmp"
  Primary path is "/private/var/tmp/index.ts" in namespace "file"
  • If I create a tsconfig with the target set to esnext it is picked up and I could run esbuild without options and get the code without a constructor.
  • If I rename the file to .js and remove the private keywords, I get the desired esnext level code that doesn't use the constructor without having to specify a target.

Is it intended that the default target esnext doesn't seem to apply to TypeScript or is this a bug? I searched through other issues and found this comment: #923 (comment) which to me sounds like the esnext default should apply to TypeScript.

@evanw
Copy link
Owner

evanw commented Feb 13, 2022

This is intentional because in TypeScript specifying esnext actually has the side-effect of changing making breaking changes to the behavior of your code, which seemed like a bad idea to have as a default behavior. Setting the TypeScript compiler to esnext, which is not the default, causes the TypeScript compiler to use "define semantics" instead of "assign semantics" for class fields, which breaks code that inherits setters with the same name from the base class.

If esbuild did as you suggested, then people's working TypeScript code would be broken when building with esbuild. That's why this behavior is opt-in for esbuild. It's unfortunate that TypeScript originally went with class field semantics for their language and it's also unfortunate that they are now trying to change it (especially how subtly they are doing it). But I understand wanting to align TypeScript semantics with JavaScript semantics and don't know how they could make this transition more smooth (besides communicating more clearly about the change).

I may change this once ES2022 comes out. The way this actually works in the TypeScript compiler is that if useDefineForClassFields is unspecified, it will be true if the target is ES2022 or higher and false otherwise. Except there's actually no ES2022 target yet so the only target affected by this is esnext, which is an unstable target. Once TypeScript releases their ES2022 target then this behavior will be stabilized and it could make sense for esbuild to switch it's default behavior to match the latest stable TypeScript language target. But changing esbuild's default behavior while this hasn't stabilized yet seems like perhaps not the best idea.

@evanw evanw added the breaking label Feb 13, 2022
@bennypowers
Copy link

bennypowers commented Mar 2, 2022

with typescript 4.6, there's es2022 now, and if it's set in tsconfig then

⚠️ [WARNING] Unrecognized target environment "es2022"

@evanw
Copy link
Owner

evanw commented Mar 3, 2022

That's really weird. There is no ES2022 yet: https://tc39.es/ecma262/2022/ returns 404 (see https://tc39.es/ecma262/2021/ and earlier for comparison).

@bennypowers
Copy link

That might be the case, but as far as typescript is concerned....
https://devblogs.microsoft.com/typescript/announcing-typescript-4-6/#target-es2022

evanw added a commit that referenced this issue Mar 3, 2022
@evanw
Copy link
Owner

evanw commented Mar 3, 2022

Ok. I added an es2022 target for esbuild and guessed at what it should contain. We can update it later if it turns out to be wrong. It's slightly different than TypeScript's since TypeScript doesn't support a feature that will likely be in ES2022 while esbuild does (the arbitrary module namespace names feature).

@anzorb
Copy link

anzorb commented Jan 19, 2023

@evanw Is there anything else needed, now that https://tc39.es/ecma262/2022/ is available? Or are we safe using ES2022 as the target? Thanks!

@evanw
Copy link
Owner

evanw commented Jun 9, 2023

This is going to be fixed in the next release. In the next release, this will be both esbuild's default behavior (without any flags) and esbuild's behavior with --target=esnext:

class MyClass {
  _propertyOne = 1;
  _propertyTwo = 2;
  printProperties() {
    console.log(this._propertyOne, this._propertyTwo);
  }
}
new MyClass().printProperties();

and this will be esbuild's behavior with --target=es2021:

var __defProp = Object.defineProperty;
var __defNormalProp = (obj, key, value) => key in obj ? __defProp(obj, key, { enumerable: true, configurable: true, writable: true, value }) : obj[key] = value;
var __publicField = (obj, key, value) => {
  __defNormalProp(obj, typeof key !== "symbol" ? key + "" : key, value);
  return value;
};
class MyClass {
  constructor() {
    __publicField(this, "_propertyOne", 1);
    __publicField(this, "_propertyTwo", 2);
  }
  printProperties() {
    console.log(this._propertyOne, this._propertyTwo);
  }
}
new MyClass().printProperties();

In other words, esbuild will now always use define semantics for class fields by default. You will now have to explicitly configure esbuild to use assign semantics using tsconfig.json. For example, this will be esbuild's behavior when you pass --tsconfig-raw='{"compilerOptions":{"useDefineForClassFields":false}}' (you can also put useDefineForClassFields in a tsconfig.json file and esbuild will pick it up automatically):

class MyClass {
  constructor() {
    this._propertyOne = 1;
    this._propertyTwo = 2;
  }
  printProperties() {
    console.log(this._propertyOne, this._propertyTwo);
  }
}
new MyClass().printProperties();

So I consider this issue to be fixed.

@evanw evanw closed this as completed Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants