Skip to content

[Transforms] Fixes for ShorthandPropertyAssignments and exported namespaces#8233

Merged
rbuckton merged 2 commits into
transformsfrom
transforms-fixShortHandPropertiesAndExportedModules
Apr 22, 2016
Merged

[Transforms] Fixes for ShorthandPropertyAssignments and exported namespaces#8233
rbuckton merged 2 commits into
transformsfrom
transforms-fixShortHandPropertiesAndExportedModules

Conversation

@rbuckton
Copy link
Copy Markdown
Contributor

This change revises how JIT substitution works in the printer. Previously we had two hooks for substitution on TransformationContext: identifierSubstitution and expressionSubstitution. Now, we instead have a single hook, onSubstituteNode, which adds an additional isExpression parameter that will be provided a value indicating whether the node to be substituted is expected to be used as an expression.

This change allows us to easily add substitutions for a node that is neither an identifier nor an expression, such as a ShorthandPropertyAssignment.

Fixes #7862.

Fixes the following tests:

  • tests/cases/conformance/es6/destructuring/declarationsAndAssignments.ts
  • tests/cases/conformance/es6/shorthandPropertyAssignment/objectLiteralShorthandPropertiesWithModuleES6.ts
  • tests/cases/conformance/internalModules/moduleDeclarations/invalidInstantiatedModule.ts
  • tests/cases/compiler/giant.ts
  • tests/cases/compiler/privacyImportParseErrors.ts
  • tests/cases/compiler/shorthand-property-es6-amd.ts
  • tests/cases/compiler/shorthandPropertyAssignmentInES6Module.ts

Comment thread src/compiler/printer.ts Outdated
if (substitution && (getNodeEmitFlags(node) & NodeEmitFlags.NoSubstitution) === 0) {
const substitute = substitution(node);
function tryEmitSubstitute(node: Node, emitNode: (node: Node) => void, isExpression: boolean) {
if ((<any>node).text === "localizedDiagnosticMessages" && isExpression) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not think you need this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks. Left-over from trying to fix an issue.

@sandersn
Copy link
Copy Markdown
Member

👍

@rbuckton rbuckton added the Domain: API: Transforms Relates to the public transform API label Apr 22, 2016
@rbuckton rbuckton merged commit 43914ff into transforms Apr 22, 2016
@rbuckton rbuckton deleted the transforms-fixShortHandPropertiesAndExportedModules branch April 22, 2016 17:57
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Domain: API: Transforms Relates to the public transform API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants