Skip to content

Comments

fix(ast): put decorators before everything else.#4143

Merged
graphite-app[bot] merged 1 commit intomainfrom
07-09-fix_reorder_every_instance_of_decorators_
Jul 10, 2024
Merged

fix(ast): put decorators before everything else.#4143
graphite-app[bot] merged 1 commit intomainfrom
07-09-fix_reorder_every_instance_of_decorators_

Conversation

@rzvxa
Copy link
Contributor

@rzvxa rzvxa commented Jul 9, 2024

Won't fix #4142

It is similar to #3994 but for those types that weren't relying on this order. It seems to be the right order.
technically speaking it is a breaking change but I know as a fact that it won't have a big difference on our downstream. If you want it to be chronically correct feel free to merge as a breaking change.

@github-actions github-actions bot added the A-ast Area - AST label Jul 9, 2024
Copy link
Contributor Author

rzvxa commented Jul 9, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @rzvxa and the rest of your teammates on Graphite Graphite

@github-actions github-actions bot added A-parser Area - Parser A-transformer Area - Transformer / Transpiler A-isolated-declarations Isolated Declarations labels Jul 9, 2024
@rzvxa rzvxa marked this pull request as ready for review July 9, 2024 18:23
@rzvxa rzvxa requested review from Dunqing and overlookmotel July 9, 2024 18:23
@rzvxa rzvxa changed the title fix: reorder every instance of decorators. fix(ast): reorder every instance of decorators. Jul 9, 2024
@rzvxa rzvxa changed the title fix(ast): reorder every instance of decorators. fix(ast): put all decorators before everything else. Jul 9, 2024
@rzvxa rzvxa changed the title fix(ast): put all decorators before everything else. fix(ast): put decorators before everything else. Jul 9, 2024
@codspeed-hq
Copy link

codspeed-hq bot commented Jul 9, 2024

CodSpeed Performance Report

Merging #4143 will not alter performance

Comparing 07-09-fix_reorder_every_instance_of_decorators_ (a536cf5) with main (5731e39)

Summary

✅ 29 untouched benchmarks

Copy link
Member

@overlookmotel overlookmotel left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@Boshen
Copy link
Member

Boshen commented Jul 10, 2024

LOVE the fact that we don't need to touch all the boilerplate code anymore, it wasn't even possible last week.

@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Jul 10, 2024
@graphite-app
Copy link
Contributor

graphite-app bot commented Jul 10, 2024

Merge activity

  • Jul 9, 10:02 PM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 9, 10:02 PM EDT: Boshen added this pull request to the Graphite merge queue.
  • Jul 9, 10:07 PM EDT: Boshen merged this pull request with the Graphite merge queue.

Won't fix #4142

It is similar to #3994 but for those types that weren't relying on this order. It seems to be the right order.
technically speaking it is a breaking change but I know as a fact that it won't have a big difference on our downstream. If you want it to be chronically correct feel free to merge as a breaking change.
@Boshen Boshen force-pushed the 07-09-fix_reorder_every_instance_of_decorators_ branch from a536cf5 to 48947a2 Compare July 10, 2024 02:03
@graphite-app graphite-app bot merged commit 48947a2 into main Jul 10, 2024
@graphite-app graphite-app bot deleted the 07-09-fix_reorder_every_instance_of_decorators_ branch July 10, 2024 02:07
@github-actions github-actions bot mentioned this pull request Jul 11, 2024
Boshen added a commit that referenced this pull request Jul 11, 2024
## [0.20.0] - 2024-07-11

- 5731e39 ast: [**BREAKING**] Store span details inside comment struct
(#4132) (Luca Bruno)

### Features

- 67fe75e ast, ast_codegen: Pass the `scope_id` to the `enter_scope`
event. (#4168) (rzvxa)
- 54cd04a minifier: Implement dce with var hoisting (#4160) (Boshen)
- 44a894a minifier: Implement return statement dce (#4155) (Boshen)
- 725571a napi/transformer: Add `jsx` option to force parsing with jsx
(#4133) (Boshen)

### Bug Fixes

- 48947a2 ast: Put `decorators` before everything else. (#4143) (rzvxa)
- 7a059ab cfg: Double resolution of labeled statements. (#4177) (rzvxa)
- 4a656c3 lexer: Incorrect lexing of large hex/octal/binary literals
(#4072) (DonIsaac)
- 28eeee0 parser: Fix asi error diagnostic pointing at invalid text
causing crash (#4163) (Boshen)

### Performance

- ddfa343 diagnostic: Use `Cow<'static, str>` over `String` (#4175)
(DonIsaac)
- 2203143 semantic: Store unresolved refs in a stack (#4162) (lucab)
- fca9706 semantic: Faster search for leading comments (#4140) (Boshen)

### Documentation

- bdcc298 ast: Update the note regarding the `ast_codegen` markers.
(#4149) (rzvxa)

### Refactor

- 03ad1e3 semantic: Tweak comment argument type (#4157) (lucab)

Co-authored-by: Boshen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0-merge Merge with Graphite Merge Queue A-ast Area - AST A-isolated-declarations Isolated Declarations A-parser Area - Parser A-transformer Area - Transformer / Transpiler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

must visit decorators before enter_node and enter_scope if have decorators field

3 participants