proposal-decorators icon indicating copy to clipboard operation
proposal-decorators copied to clipboard

Dangling decorationState.[[Finished]] when throwing

Open camillobruni opened this issue 1 year ago • 9 comments

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

camillobruni avatar Mar 07 '23 14:03 camillobruni

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

pzuraq avatar Feb 02 '24 19:02 pzuraq

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

ljharb avatar Feb 02 '24 19:02 ljharb

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

nicolo-ribaudo avatar Feb 07 '24 21:02 nicolo-ribaudo

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

pzuraq avatar Feb 08 '24 13:02 pzuraq

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

ljharb avatar Feb 08 '24 16:02 ljharb

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

pzuraq avatar Feb 08 '24 16:02 pzuraq

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

nicolo-ribaudo avatar Feb 08 '24 16:02 nicolo-ribaudo

@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.

pzuraq avatar Feb 08 '24 16:02 pzuraq

Ok well, I opened https://github.com/pzuraq/ecma262/pull/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).

nicolo-ribaudo avatar Feb 08 '24 18:02 nicolo-ribaudo