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

Dangling decorationState.[[Finished]] when throwing #500

Open
camillobruni opened this issue Mar 7, 2023 · 9 comments
Open

Dangling decorationState.[[Finished]] when throwing #500

camillobruni opened this issue Mar 7, 2023 · 9 comments

Comments

@camillobruni
Copy link

camillobruni commented Mar 7, 2023

If a decorator throws, we don't set decorationState.[[Finished]] = true.
This means that if addInitializerClosure is used later, it can still be called without throwing which might be a bit misleading.

This applies to the two following steps:

  • 15.7.6 ApplyDecoratorsToElementDefinition, step 5.h
  • 15.7.7 ApplyDecoratorsToClassDefinition , step 1.c
@pzuraq
Copy link
Collaborator

pzuraq commented Feb 2, 2024

@ljharb would this be a normative change to fix? Definitely was not the intended behavior

@ljharb
Copy link
Member

ljharb commented Feb 2, 2024

Yes, but it doesn't seem like a controversial one.

@nicolo-ribaudo
Copy link
Member

I also stumbled upon this in the Babel implementation, I'd love for it to be changed.

@pzuraq
Copy link
Collaborator

pzuraq commented Feb 8, 2024

It definitely will, I'll propose it at the upcoming plenary, just needed to figure out if it was normative

@ljharb
Copy link
Member

ljharb commented Feb 8, 2024

You could probably fit it into today’s plenary session if you’re available.

@pzuraq
Copy link
Collaborator

pzuraq commented Feb 8, 2024

Unfortunately I am not available today, my schedule is packed at the moment 😕

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Feb 8, 2024

@pzuraq I can present it for you of you want (this change)

@pzuraq
Copy link
Collaborator

pzuraq commented Feb 8, 2024

@nicolo-ribaudo I don't have a PR or slides for it unfortunately. The change is just to make the procedure that calls decorators be an explicit completion that catches the error and sets the didDecorate state to true, so it can't be called again. It's not a huge change, I just don't have time to handle it today.

@nicolo-ribaudo
Copy link
Member

Ok well, I opened pzuraq/ecma262#13 in the meanwhile. If you change your mind and you are ok with me presenting just ping me :) (this change is so small that the PR description is enough, it doesn't need slides).

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

4 participants