search

oxc-project / oxc

⚓ A collection of JavaScript tools written in Rust.
https://oxc.rs
MIT License
10.69k stars 394 forks source link

parser: add regex AST node #5060

Closed Boshen closed 2 weeks ago

Boshen commented 4 weeks ago

https://github.com/oxc-project/oxc/blob/afe728a73a34fc81f8e9de57bd8089f1eabc0f9e/crates/oxc_parser/src/js/expression.rs#L348-L351

This pattern node cannot be added to the AST due to not having CloneIn implemented. cc @rzvxa

rzvxa commented 3 weeks ago

@Boshen Do we want these types to be feature-gated? If so It seems this would take more time than I initially expected. It requires me to go in and refactor stuff related to our AST layout to support conditional layouts. It would also mean that we have to distinguish between these layouts when doing AST transfer but it isn't too hard to support, It just takes more time as we need to represent conditional fields and type definitions.

Boshen commented 3 weeks ago

@Boshen Do we want these types to be feature-gated? If so It seems this would take more time than I initially expected. It requires me to go in and refactor stuff related to our AST layout to support conditional layouts. It would also mean that we have to distinguish between these layouts when doing AST transfer but it isn't too hard to support, It just takes more time as we need to represent conditional fields and type definitions.

Welcome back ❤️

No feature gate is needed, all the regular expression parsing code is really lightweight.

rzvxa commented 3 weeks ago

Welcome back ❤️

Thanks Captain!⚓

No feature gate is needed, all the regular expression parsing code is really lightweight.

That's great to hear, It Simplifies the process and doesn't prevent us from adding it later on as a follow-up PR. If you would like to feature-gate it in the future, Let me know so I can create an issue for it in the backlog.

rzvxa commented 2 weeks ago

Should we close this issue?

rzvxa commented 2 weeks ago

I'll close it since #5256 addresses this. Feel free to reopen if you think the requirements for this issue aren't met yet.