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

Remove HIR #917

Merged
merged 1 commit into from
Sep 15, 2023
Merged

Remove HIR #917

merged 1 commit into from
Sep 15, 2023

Conversation

Boshen
Copy link
Member

@Boshen Boshen commented Sep 15, 2023

closes #273
closes #814

HIR is removed from this PR, with the minifier being commented out.

HIR is a wonderful idea for compiling to lower languages, but after sitting on it for a few months I found that it only adds confusion and uncertainties to both myself and future contributors.

It also adds too much burden to maintainers if we plan to support more downstream tools.

1 AST is the only way.

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 15, 2023

CodSpeed Performance Report

Merging #917 will not alter performance

Comparing remove-hir (c59edb8) with main (38bbc96)

Summary

✅ 14 untouched benchmarks

⁉️ 4 (👁 4) dropped benchmarks

Benchmarks breakdown

Benchmark main remove-hir Change
👁 minifier[antd.js] 3.1 s N/A N/A
👁 minifier[typescript.js] 4.7 s N/A N/A
👁 minifier[react.development.js] 36.1 ms N/A N/A
👁 minifier[vue.js] 222.8 ms N/A N/A

HIR is removed from this PR, with the minifier being commented out.

HIR is a wonderful idea for compiling to lower languages, but after
sitting on it for a few months I found that it only adds confusion and
uncertainties to both myself and future contributors.

It also adds too much burden to maintainers if we plan to support more
downstream tools.

1 AST is the only way.
@Boshen Boshen merged commit ceeee59 into main Sep 15, 2023
@Boshen Boshen deleted the remove-hir branch September 15, 2023 15:48
@magic-akari
Copy link
Contributor

We need an article about it!

@Boshen
Copy link
Member Author

Boshen commented Sep 15, 2023

We need an article about it!

On the technical aspects or why it is removed?

@marvinhagemeister
Copy link

+1 for an article, or a short tweet thread. I'm mostly interested in your experiences like what the hope was, what worked, what didn't work out and concluding with the removal of HIR.

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

Successfully merging this pull request may close these issues.

HIR is a blocker for working on the parser.
3 participants