feat(pluginutils): add staticBlock(es2022) support#2000
Conversation
|
@CharlieHelps please do a thorough review of this PR to determine if it's ready to merge. Look at the diff, reviews, comments, and referenced issues/docs/links to build a complete understanding. Then respond with a comment that includes a list of MUST fix items that must be fixed before the the PR is merged, and a list of IMPROVEMENTS that shouldn't block the merge but may be quick improvements or things that could be addressed in a follow up PR. The MUST fix items should explain exactly what's wrong and what needs to change to resolve the problem. Both should be numbered lists and the IMPROVEMENTS list should continue counting up from the MUST list. It's OK if there are no IMPROVEMENTS or MUST fix items, you can just omit the lists. |
There was a problem hiding this comment.
Non-blocking feedback (1)
- Add a
varregression assertion for static-block scoping — packages/pluginutils/test/attachScopes.ts#L76-L96
The implementation change is specifically about givingStaticBlockfunction-like scope behavior (block: false), which mainly affectsvardeclarations. The new test currently useslet, so it would not catch a regression where static blocks are treated as plain block scope. Please add avarcase (ideally includingvarinside a nested block) and assert it is contained bystaticBlock.scopebut not the top-level scope.
If you want me to push fixes, reply with item numbers (for example: please fix 1).
|
@shellscape I completed a thorough review of this PR (diff, existing discussion/reviews, CI status, and relevant docs). I do not see any merge-blocking issues. From a code-quality and correctness standpoint, this looks ready to merge. IMPROVEMENTS
|
|
@linsk1998 I think it's worth adding the test that Charlie mentioned. seems very prudent. Please do so and we'll merge this. |
Rollup Plugin Name:
pluginutilsThis PR contains:
Are tests included?
Breaking Changes?
If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.
List any relevant issue numbers:
Description