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

Improve super.x output #16374

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Improve super.x output #16374

wants to merge 11 commits into from

Conversation

liuxingbaoyu
Copy link
Member

@liuxingbaoyu liuxingbaoyu commented Mar 21, 2024

Q                       A
Fixed Issues? Fixes #1, Fixes #2
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

I personally don't like having to call getPrototypeOf every time because it's slow, but we have a test that relies on this behavior.
image

@babel-bot
Copy link
Collaborator

babel-bot commented Mar 21, 2024

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/57350

@liuxingbaoyu liuxingbaoyu marked this pull request as draft March 22, 2024 00:36
@liuxingbaoyu liuxingbaoyu marked this pull request as ready for review March 22, 2024 01:14
@liuxingbaoyu liuxingbaoyu added the PR: Output optimization 🔬 A type of pull request used for our changelog categories label Mar 22, 2024
@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Jun 21, 2024

Finally reviewing this, sorry it took so long.

I personally don't like having to call getPrototypeOf every time because it's slow, but we have a test that relies on this behavior.

This is covered by the constantSuper assumption, right?


Can you add a test for the execution order between property access and arguments evaluation?

let argsEval = false;
let err = {};

class A {
  get x() { throw {} }
}

class B extends A {
  method() {
    super.x(argsEval = true);
  }
}

expect(() => new B().method()).toThrow(err);
expect(argsEval).toBe(false);

@liuxingbaoyu liuxingbaoyu force-pushed the super.prop branch 2 times, most recently from dd1bc89 to 92e2168 Compare June 25, 2024 06:34
@liuxingbaoyu liuxingbaoyu force-pushed the super.prop branch 2 times, most recently from 9c609ac to 8cb4293 Compare July 13, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Output optimization 🔬 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants