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

Fix Super Calls #2839

Closed
wants to merge 2 commits into from
Closed

Fix Super Calls #2839

wants to merge 2 commits into from

Conversation

vixalien
Copy link
Contributor

@vixalien vixalien commented Nov 8, 2023

What:

Fixes super calls like super.method.call(this, ...args) by replacing them with direct super calls like super.method(...args)

Why:

I did this change to avoid the .call pattern.

It also fixes #2820 which is a regression caused by the Classes PR (#2813) where code was transformed incorrectly on my part, which I apologise for.

The code before the PR was:

Node.prototype.operate.call(this, op, right)

and the code was incorrectly transformed into

super.operate.call(op, right);

when it should have instead been

super.operate(op, right);

Checklist:

  • Documentation
  • Unit Tests
  • Code complete
  • Changelog

the super.operate.call syntax doesn't pass in the `this` argument.

The code before the classes #2813 PR was:

```js
Node.prototype.operate.call(this, op, right)
```

and the code was incorrectly transformed into

```js
super.operate.call(op, right);
```
instead of doing `super.method.call(this, ...args)`
Copy link

netlify bot commented Nov 8, 2023

Deploy Preview for stylus-docs canceled.

Name Link
🔨 Latest commit 1318e7c
🔍 Latest deploy log https://app.netlify.com/sites/stylus-docs/deploys/654ba46e443646000885eb5f

@iChenLei
Copy link
Member

iChenLei commented Nov 9, 2023

Thank you, @vixalien . Can you provide some additional test cases to avoid regression #2820 ?

@vixalien
Copy link
Contributor Author

Hi. I'm not sure how to approach this. It would be a good idea to add a test case for the regression, but it might be a better idea to add a test case that checks the is a operator?

How can I approach this?

iChenLei added a commit that referenced this pull request Nov 11, 2023
This was referenced Nov 11, 2023
@iChenLei
Copy link
Member

closed via #2843

@iChenLei iChenLei closed this Nov 18, 2023
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

Successfully merging this pull request may close these issues.

0.60.0 breaks basic type check
2 participants