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

[Bug] browserslist and targets have no effect on the output of the build (potential performance problems as well) #19790

Closed
NullVoxPopuli opened this issue Oct 14, 2021 · 3 comments

Comments

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Oct 14, 2021

🐞 Describe the Bug

After some investigation here: https://stackoverflow.com/questions/69547969/with-babel-how-do-i-not-compile-away-class-properties-since-browsers-support
And a minimal babel repro here: https://github.com/NullVoxPopuli/babel-transpilation-tests
I expect that setting my targets to last 1 firefox versions would allow me to keep native class properties. and only transpile what is decororated.

I believe fixing this could provide a bit of a boost to everyone's build times. Today all class properties are transpiled to a time when class properties were not implemented in the browser).

🔬 Minimal Reproduction

  • ember new my-app
  • cd my-app
  • remove safari from targets, leaving only last 1 Chrome versions and last 1 firefox versions
  • add browserslist to package.json
  • add browserslist as devDependency
  • yarn browserslist --update-db
  • run ember build -e production
  • in the my-app.js, I expect all occurrences of rootURL to be involved in an assignment, rather than a function call.

result: https://github.com/NullVoxPopuli/repro-for-emberjs-19790

😕 Actual Behavior

Living in the dark ages

🤔 Expected Behavior

Utilize support for native properties, private properties, methods, etc.

🌍 Environment

  • Ember: - CLI 3.28.0
  • Node.js/npm: -14.17.6
  • OS: - n/a
  • Browser: - n/a

➕ Additional Context

See the comparison between these two files:

Input: https://github.com/NullVoxPopuli/babel-transpilation-tests/blob/main/input.js
Output: https://github.com/NullVoxPopuli/babel-transpilation-tests/blob/main/output.js

This is correct.
But when I add that input file to ember, I get this for the output:

/home/nullvoxpopuli/Development/tmp/my-app/my-app/input.js: Class private methods are not enabled.
  2 |   #a = 'hi';
  3 |
> 4 |   get #b() {
    |   ^
  5 |     return 'hello' + this.#a;
  6 |   }
  7 |

which... I guess is unrelated, but should still be fixed. Now, If I change the private methods/getters to public:
I get this mess:

  var e, t, r
  function n(e, t, r) {(function (e, t) {if (t.has(e)) throw new TypeError("Cannot initialize the same private elements twice on an object")})(e, t), t.set(e, r)} function i(e, t) {
    var r = function (e, t, r) {
      if (!t.has(e)) throw new TypeError("attempted to " + r + " private field on non-instance")
      return t.get(e)
    }(e, t, "get")
    return function (e, t) {
      if (t.get) return t.get.call(e)
      return t.value
    }(e, r)
  } r = new WeakMap, e = class {
    constructor() {
      var e, i, a, l
      n(this, r, {writable: !0, value: "hi"}), e = this, i = "g", l = this, (a = t) && Object.defineProperty(e, i, {enumerable: a.enumerable, configurable: a.configurable, writable: a.writable, value: a.initializer ? a.initializer.call(l) : void 0})
    } get b() {return "hello" + i(this, r)} get c() {return this.b}
  }, a = e.prototype, l = "g", o = [f], u = {configurable: !0, enumerable: !0, writable: !0, initializer: null}, p = {}, Object.keys(u).forEach((function (e) {p[e] = u[e]})), p.enumerable = !!p.enumerable, p.configurable = !!p.configurable, ("value" in p || p.initializer) && (p.writable = !0), p = o.slice().reverse().reduce((function (e, t) {return t(a, l, e) || e}), p), d && void 0 !== p.initializer && (p.value = p.initializer ? p.initializer.call(d) : void 0, p.initializer = void 0), void 0 === p.initializer && (Object.defineProperty(a, l, p), p = null), t = p
  var a, l, o, u, d, p

which is way wrong.

@NullVoxPopuli
Copy link
Contributor Author

Moved to: emberjs/ember-cli-babel#417

@kategengler
Copy link
Member

I was able to reproduce the ember output in babel-transpilation-tests by passing modules: false to preset-env (as ember-cli-babel does).

@NullVoxPopuli
Copy link
Contributor Author

💯 I replied over in the ember-cli-babel issue

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