Skip to content

fix: tagged template incorrect receiver#13395

Merged
JLHwung merged 6 commits intobabel:mainfrom
sag1v:tagged-private-field-this
Jun 15, 2021
Merged

fix: tagged template incorrect receiver#13395
JLHwung merged 6 commits intobabel:mainfrom
sag1v:tagged-private-field-this

Conversation

@sag1v
Copy link
Copy Markdown
Contributor

@sag1v sag1v commented May 29, 2021

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

@babel-bot
Copy link
Copy Markdown
Collaborator

babel-bot commented May 29, 2021

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

@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci Bot commented May 29, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 9a0d7ec:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

@nicolo-ribaudo nicolo-ribaudo added PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Class Fields Spec: Private Methods labels May 29, 2021
@sag1v
Copy link
Copy Markdown
Contributor Author

sag1v commented May 29, 2021

@nicolo-ribaudo Thanks for the comments.

General question, Can i create a new commit (fixing review) and push to this PR or do i must keep it with a single commit (using commit --amend with force push)?

Comment thread packages/babel-helper-member-expression-to-functions/src/index.ts Outdated
@jridgewell
Copy link
Copy Markdown
Member

It's fine to add as many commits as needed to this PR.

@sag1v
Copy link
Copy Markdown
Contributor Author

sag1v commented May 29, 2021

@nicolo-ribaudo @jridgewell I pushed the new changes per your comments

@sag1v
Copy link
Copy Markdown
Contributor Author

sag1v commented Jun 1, 2021

@nicolo-ribaudo / @jridgewell Are we done here? :)

Copy link
Copy Markdown
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a few more comments about tests; apart from them it looks good 👍

@sag1v sag1v requested a review from jridgewell June 12, 2021 10:54
Copy link
Copy Markdown
Member

@jridgewell jridgewell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, just a few more comments (that I screwed up). If you can make these changes, run update the tests one last time, this should be good to merge.

@sag1v
Copy link
Copy Markdown
Contributor Author

sag1v commented Jun 15, 2021

@jridgewell Thanks, done.

Copy link
Copy Markdown
Member

@existentialism existentialism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@JLHwung JLHwung merged commit a3c7497 into babel:main Jun 15, 2021
@github-actions github-actions Bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Sep 15, 2021
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Sep 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Class Fields Spec: Private Methods

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Tagged template with strict private field/method tag has incorrect receiver

6 participants