Skip to content

Fix shorthand properties for non-es6 module formats#7442

Merged
RyanCavanaugh merged 3 commits into
microsoft:masterfrom
Victorystick:fix-property-shorthand-emit
Mar 9, 2016
Merged

Fix shorthand properties for non-es6 module formats#7442
RyanCavanaugh merged 3 commits into
microsoft:masterfrom
Victorystick:fix-property-shorthand-emit

Conversation

@Victorystick
Copy link
Copy Markdown

Shorthand properties like { x } are currently expanded to { x: x } when using any module format other than ES6. This PR changes this to apply to an ES6+ script target instead.

The special cases considered now also includes whether the current module format has direct or indirect access to imported identifiers. The ES6 and System formats for example have direct access, all others have indirect access. This means that the shorthand properties must be expanded, even for ES6+ script targets if the identifier is indirectly imported.

For example, the object expression in code below can be preserved as is when targeting ES6+ with an ES6 or System module format.

Update: After discovering that TypeScript doesn't generate direct identifier accesses for the System module format, the changes now only affects ES6 modules.

import { foo } from './foo';
export const bar = { foo };
// es6
import { foo } from './foo';
export const bar = { foo };

but is expanded for all other formats.

// system
System.register(['./foo'], function(exports_1, context_1) {
    "use strict";
    var __moduleName = context_1 && context_1.id;
    var foo_1;
    var bar;
    return {
        setters:[
            function (foo_1_1) {
                foo_1 = foo_1_1;
            }],
        execute: function() {
            exports_1("bar", bar = { foo: foo_1.foo });
        }
    }
});

// cjs
"use strict";
const foo_1 = require('./foo');
exports.bar = { foo: foo_1.foo };

// amd
define(["require", "exports", './foo'], function (require, exports, foo_1) {
    "use strict";
    exports.bar = { foo: foo_1.foo };
});

Fixes #7434

@msftclas
Copy link
Copy Markdown

msftclas commented Mar 9, 2016

Hi @Victorystick, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

@RyanCavanaugh
Copy link
Copy Markdown
Member

To add a testcase:

  • Create a new file in tests/cases/compiler/
  • Add appropriate settings directives to the top of the file (see es3-declaration-amd.ts for an example)
  • jake runtests
  • jake baseline-accept
  • Push a commit adding the new files that will now be in tests/baselines/reference

@Victorystick
Copy link
Copy Markdown
Author

@RyanCavanaugh Is that alright?

@RyanCavanaugh
Copy link
Copy Markdown
Member

Yep, that looks right

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Mar 9, 2016

👍

RyanCavanaugh added a commit that referenced this pull request Mar 9, 2016
Fix shorthand properties for non-es6 module formats
@RyanCavanaugh RyanCavanaugh merged commit ac147b1 into microsoft:master Mar 9, 2016
@RyanCavanaugh
Copy link
Copy Markdown
Member

Thanks!

@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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants