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

SES compatibility for '--emitDecoratorMetadata' #43463

Open
4 of 5 tasks
rbuckton opened this issue Mar 31, 2021 · 3 comments Β· May be fixed by #43542
Open
4 of 5 tasks

SES compatibility for '--emitDecoratorMetadata' #43463

rbuckton opened this issue Mar 31, 2021 · 3 comments Β· May be fixed by #43542
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@rbuckton
Copy link
Member

Suggestion

πŸ” Search Terms

ses reflect metadata decorator emitDecoratorMetadata experimentalDecorators

βœ… Viability Checklist

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

❗ Problem

SES (Secure ECMAScript) is an initiative to create a locked-down and sandboxed JavaScript execution environment. One of the tenets of SES is that "primordials" (built-in/native ECMAScript global objects, prototypes, functions, etc.) should be immutable and be locked down to a specific set of APIs. This set of APIs includes those specified in the ECMAScript standard and other "blessed" APIs depending on the runtime environment.

A longstanding package used by many in the community is reflect-metadata, which is used with TypeScript's --emitDecoratorMetadata compiler flag and the __metadata helper:

Example:

tsc example.ts \
  --target esnext \
  --experimentalDecorators \
  --emitDecoratorMetadata

example.ts

declare const dec: any;
@dec
class C {
  constructor(x: int) {
  }
}

example.js

var __decorate = ...
var __metadata = (this && this.__metadata) || function (k, v) {
    if (typeof Reflect === "object" && typeof Reflect.metadata === "function") return Reflect.metadata(k, v);
};
let C = class C {
  constructor(x) {
  }
};
C = __decorate([
  dec,
  __metadata("design:paramtypes", [Number])
], C);

The issue is that the __metadata helper function explicitly relies on an early draft of a proposal that has not yet been brought to committee (as it depends on the decorators proposal, which has not yet reached Stage 3). Unfortunately, reflect-metadata mutates the global Reflect object to shim its API, which violates SES. This was recently brought up in endojs/endo#612 and brought to my attention by @erights.

Alternatives to reflect-metadata exist that do not rely on global mutation, such as https://esfx.js.org/esfx/api/metadata.html. However, they cannot be easily used with TypeScript's --emitDecoratorMetadata since our helper is hardcoded to check for the specific global. While it is possible to overwrite the helper locally by providing your own __metadata helper, this is behavior is intentionally undocumented and is also burdensome on developers.

⭐ Suggestion

I am proposing that we add two new compiler options to control how we emit decorator metadata:

  • --metadataDecorator <string> β€” Accepts a valid EntityName that should be used as the decorator in place of __metadata
    • This option can only be specified if both --experimentalDecorators and --emitDecoratorMetadata are set.
  • --metadataDecoratorImportSource <string> β€” A module specifier used to import the provided metadata decorator.
    • This option can only be specified if --experimentalDecorators, --emitDecoratorMetadata, and --metadataDecorator are set.

NOTE: These two options are somewhat analogous to our existing --jsxFactory and --jsxImportSource options.

--metadataDecorator option

If the --metadataDecorator option is set without specifying --metadataDecoratorImportSource, then the following occurs:

  • During command line validation we verify that the provided option is a valid EntityName.
  • During check we verify that the provided name can be resolved to a value that can be called as a decorator appropriate to the element on which it would appear as a result of --emitDecoratorMetadata.
  • During emit, we would emit the provided EntityName in place of __metadata.

NOTE: This scenario assumes that the provided metadata decorator exists as a global, or is manually imported into any module.

Example:

tsc example.ts \
  --target esnext \
  --experimentalDecorators \
  --emitDecoratorMetadata \
  --metadataDecorator myCustomMetadata

example.ts

declare const dec: any;

@dec
class C {
  constructor(x: number) {}
}

example.js

var __decorate = ...
let C = class C {
  constructor(x) {
  }
}
C = __decorate([
  dec,
  myCustomMetadata("design:paramtypes", [Number])
], C);

--metadataDecoratorImportSource option

If the --metadataDecoratorImportSource option is set along with --metadataDecorator, then the following occurs:

  • During command line validation we verify that the provided option is a valid EntityName.
  • During check we verify that the provided import source can be resolved as a module.
  • During check we verify that the first identifier in the provided EntityName is an export of the import source.
  • During check we verify that the remainder of the provided EntityName can be resolved to a value relative to the imported binding above, that can be called as a decorator appropriate to the element on which it would appear as a result of --emitDecoratorMetadata.
  • During emit, we would emit a synthetic import statement for the import source.
  • During emit, we would emit the provided EntityName in place of __metadata.

Example:

tsc example.ts \
  --target esnext \
  --experimentalDecorators \
  --emitDecoratorMetadata \
  --metadataDecorator myCustomMetadata \
  --metadataDecoratorImportSource my-custom-metadata

example.ts

declare const dec: any;

@dec
export export class C {
  constructor(x: number) {}
}

example.js

var __decorate = ...
import { myCustomMetadata } from "my-custom-metadata"
let C = class C {
  constructor(x) {
  }
}
C = __decorate([
  dec,
  myCustomMetadata("design:paramtypes", [Number])
], C);
export { C }

Caveats

Specifying --metadataDecoratorImportSource implies that the custom metadata decorator can only be reached from within a module, therefore any source file that expects metadata must have at least one import or export declaration, or the --isolatedModules option must be set. If a file is not determined to be a module, we will fall back to the existing __metadata helper (which is essentially a no-op if the Reflect.metadata function does not exist at runtime).

Out-of-scope

  • Further changes to type metadata emit related to --emitDecoratorMetadata are out of scope. This is only intended to address an ecosystem concern brought to us by those involved with SES.
  • Changes to decorator emit related to the ongoing work in the Decorators proposal are out of scope. We do not intend to make any substantial changes to our decorator support until that proposal has reached Stage 3.

Related Issues

@erights
Copy link

erights commented Mar 31, 2021

Hi @rbuckton thanks for writing this up!

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Apr 7, 2021

One of the things we're feeling shy about is adding more compiler configuration for a feature that may change long-term - specifically, we don't like the idea of adding flags for decorators and decorator metadata when the proposal still hasn't reached stage 3. We fear this would cause churn.

An alternative work-around might be to rely on a slightly modified tslib that is SES-compatible and leveraging npm and Yarn's selective dependency resolutions/overrides (i.e. the resolutions field in package.json). This requires approximately the same amount of work for an end-user, avoids problems from older libraries that use older versions of tslib, and means we wouldn't have to prematurely add a flag that we may need to keep around long-term.

@erights
Copy link

erights commented Jun 9, 2021

@rbuckton @DanielRosenwasser any progress on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants