microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.12k stars 12.5k forks source link

Feature Request: Reduce property chain in imported symbol in SystemJS #49377

Open ackava opened 2 years ago

ackava commented 2 years ago

SystemJS Module

Lets assume module "n" as following,

let n = 1;
export { n as default };
setInterval(() => n++, 1000);

Following code,

import n from "./n";
const span = document.createElement("SPAN");
document.body.appendChild(span);
setInterval(() => 
    span.textContent = n, 1000);

Results in

System.register(["./n"], function (exports_1, context_1) {
    "use strict";
    var n_1, span;
    var __moduleName = context_1 && context_1.id;
    return {
        setters: [
            function (n_1_1) {
                n_1 = n_1_1;
            }
        ],
        execute: function () {
            span = document.createElement("SPAN");
            document.body.appendChild(span);
            setInterval(() => span.textContent = n_1.default, 1000);
        }
    };
});

This is problem, every time when we refer n, internally it is referring n_1.default. Every access to imported symbol results in two step property access.

However, if we move n_1.default in the setter instead of every access, the logic remains same.

Suggestion

Code generation can be changed to,

System.register(["./n"], function (exports_1, context_1) {
    "use strict";
    var n, span;
    var __moduleName = context_1 && context_1.id;
    return {
        setters: [
            function (n_1) {
                n = n_1.default; // <-- import changed here
            }
        ],
        execute: function () {
            span = document.createElement("SPAN");
            document.body.appendChild(span);
            setInterval(() => span.textContent = n, 1000);
        }
    };
});

There are two benefits,

  1. Every imported symbol access is a direct access instead of property chain.
  2. For function call, no need to prefix it with bracket, zero and comma
  3. Name of symbol can stay consistent, reduction in source map size.

Why this is important over minification

PLEASE READ We cannot minimize this by enabling short circuit property chains as minimizing property chains creates problem other logic where minimizer does not support conditional minimizing.

Babel plugin does it correctly, https://babeljs.io/repl#?browsers=defaults%2C%20not%20ie%2011%2C%20not%20ie_mob%2011&build=&builtIns=false&corejs=false&spec=false&loose=false&code_lz=JYWwDg9gTgLgBAOzgMyhEcBEA6A9AzAbgCgBjCBAZ3krAEMkBeOAEwlIFcQBTBGbUlG50Y3AKIAbbjz4AKTAGUACgEEAcpgCUJNpxn8ARhBYBPbHTBheLAMIALYBJazaDbcUrcYAST7coAG50ErKymnCMAHxwxHBxcK4I2KIAHjA2FKJ8EYgANHAAjAAMJdpAA&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=false&sourceType=module&lineWrap=true&presets=env%2Creact%2Cstage-2%2Ctypescript&prettier=false&targets=&version=7.18.4&externalPlugins=babel-plugin-transform-es2015-modules-systemjs%406.24.1&assumptions=%7B%7D

✅ Viability Checklist

My suggestion meets these guidelines:

Grubba27 commented 2 years ago

where would someone who wants to contribuite to this change start? do you have suggestions @RyanCavanaugh ?

Grubba27 commented 2 years ago

Is it in src/compiler/transformers/module/node.ts ? to be more exact in createSettersArray

factory.createPropertyAssignment( // line: 486
     factory.createStringLiteral(idText(e.name)),
        factory.createElementAccessExpression(
              parameterName,
            factory.createStringLiteral(idText(e.propertyName || e.name))
       )
    )
);  // line: 494

It would be someting like changing this part with the execute one but I'm I little bit confused with the flow that runs in this file.

where it creates the var n_1 variable and .default for the execute part ?

edit: I was searching for the starting point

// Line: 268
            const moduleObject = factory.createObjectLiteralExpression([
                factory.createPropertyAssignment("setters",
                    createSettersArray(exportStarFunction, dependencyGroups)
                ),
                factory.createPropertyAssignment("execute",
                    factory.createFunctionExpression(
                        modifiers,
                        /*asteriskToken*/ undefined,
                        /*name*/ undefined,
                        /*typeParameters*/ undefined,
                        /*parameters*/ [],
                        /*type*/ undefined,
                        factory.createBlock(executeStatements, /*multiLine*/ true)
                    )
                )
            ], /*multiLine*/ true);