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

ESM build has classes downleveled to functions, but CJS build does not #158

Open
bradzacher opened this issue Feb 15, 2023 · 3 comments
Open

Comments

@bradzacher
Copy link

bradzacher commented Feb 15, 2023

See files published on NPM:
ESM: https://unpkg.com/browse/js-sdsl@4.3.0/dist/esm/container/OtherContainer/Queue.js

ESM Build Source
var __extends = this && this.t || function() {
    var extendStatics = function(t, i) {
        extendStatics = Object.setPrototypeOf || {
            __proto__: []
        } instanceof Array && function(t, i) {
            t.__proto__ = i;
        } || function(t, i) {
            for (var n in i) if (Object.prototype.hasOwnProperty.call(i, n)) t[n] = i[n];
        };
        return extendStatics(t, i);
    };
    return function(t, i) {
        if (typeof i !== "function" && i !== null) throw new TypeError("Class extends value " + String(i) + " is not a constructor or null");
        extendStatics(t, i);
        function __() {
            this.constructor = t;
        }
        t.prototype = i === null ? Object.create(i) : (__.prototype = i.prototype, new __);
    };
}();

import { Base } from "../ContainerBase";

var Queue = function(t) {
    __extends(Queue, t);
    function Queue(i) {
        if (i === void 0) {
            i = [];
        }
        var n = t.call(this) || this;
        n.A = 0;
        n.tt = [];
        var e = n;
        i.forEach((function(t) {
            e.push(t);
        }));
        return n;
    }
    Queue.prototype.clear = function() {
        this.tt = [];
        this.M = this.A = 0;
    };
    Queue.prototype.push = function(t) {
        var i = this.tt.length;
        if (this.A / i > .5 && this.A + this.M >= i && i > 4096) {
            var n = this.M;
            for (var e = 0; e < n; ++e) {
                this.tt[e] = this.tt[this.A + e];
            }
            this.A = 0;
            this.tt[this.M] = t;
        } else this.tt[this.A + this.M] = t;
        return ++this.M;
    };
    Queue.prototype.pop = function() {
        if (this.M === 0) return;
        var t = this.tt[this.A++];
        this.M -= 1;
        return t;
    };
    Queue.prototype.front = function() {
        if (this.M === 0) return;
        return this.tt[this.A];
    };
    return Queue;
}(Base);

export default Queue;
//# sourceMappingURL=Queue.js.map

CJS: https://unpkg.com/browse/js-sdsl@4.3.0/dist/cjs/container/OtherContainer/Queue.js

CJS Build Source
"use strict";

Object.defineProperty(exports, "t", {
    value: true
});

exports.default = void 0;

var _ContainerBase = require("../ContainerBase");

class Queue extends _ContainerBase.Base {
    constructor(t = []) {
        super();
        this.j = 0;
        this.q = [];
        const s = this;
        t.forEach((function(t) {
            s.push(t);
        }));
    }
    clear() {
        this.q = [];
        this.i = this.j = 0;
    }
    push(t) {
        const s = this.q.length;
        if (this.j / s > .5 && this.j + this.i >= s && s > 4096) {
            const s = this.i;
            for (let t = 0; t < s; ++t) {
                this.q[t] = this.q[this.j + t];
            }
            this.j = 0;
            this.q[this.i] = t;
        } else this.q[this.j + this.i] = t;
        return ++this.i;
    }
    pop() {
        if (this.i === 0) return;
        const t = this.q[this.j++];
        this.i -= 1;
        return t;
    }
    front() {
        if (this.i === 0) return;
        return this.q[this.j];
    }
}

var _default = Queue;

exports.default = _default;
//# sourceMappingURL=Queue.js.map

If I had to guess it's because the ESM build is set to target ES5:

js-sdsl/gulpfile.ts

Lines 27 to 37 in 28fe66c

gulp.task(
'esm',
() => gulpFactory(
{ globs: 'src/**/*.ts' },
'dist/esm',
{
overrideSettings: {
target: 'ES5',
module: 'ES2015',
declaration: true
}

Whereas the CJS build does not:

js-sdsl/gulpfile.ts

Lines 12 to 21 in 28fe66c

gulp.task(
'cjs',
() => gulpFactory(
{ globs: 'src/**/*.ts' },
'dist/cjs',
{
overrideSettings: {
module: 'ES2015',
declaration: true
},

@ZLY201
Copy link
Member

ZLY201 commented Feb 16, 2023

Yes, We hope that Js-sdsl can be compatible with more browsers so we set target to ES5 for ES module.

@bradzacher
Copy link
Author

All modern browsers support much higher than es5 though - esp if they truly support ESM.
For example every modern browser supports es6 classes https://caniuse.com/es6-class

Why not just leave it to the user to downlevel your code for their target environment? It's going to be very, very rare that anyone is going to consume your package on the web without passing it through a bundler first.

The issue is that anyone consuming your package from nodejs is stuck using your downleveled code if they import your package via ESM - which is a really backwards experience!

@ZLY201
Copy link
Member

ZLY201 commented Feb 20, 2023

All modern browsers support much higher than es5 though - esp if they truly support ESM. For example every modern browser supports es6 classes https://caniuse.com/es6-class

Why not just leave it to the user to downlevel your code for their target environment? It's going to be very, very rare that anyone is going to consume your package on the web without passing it through a bundler first.

The issue is that anyone consuming your package from nodejs is stuck using your downleveled code if they import your package via ESM - which is a really backwards experience!

Sounds good, we might change the building target of ESM to ES6 at version 5.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants