After studying google closure compiler, I'm leaning towards a multi-ast-pass infrastructure for the minifier.
This is one of the few places where we are going to trade maintainability over performance, given the goal of the minifier is compression size not performance.
All of the terminologies and separation of concerns are aligned with google closure compiler.
Infrastructure of `terser` and `esbuild` are not suitable for us to study nor pursuit. Their code are so tightly coupled - I failed to comprehend any of them every time I try to walk through a piece of optmization. Google closure compiler despite being written in Java, it's actually the most readable minifier out there.
To improve performance between ast passes, I envision a change detection system over a portion of the code.
The benchmark will demonstrate the performance regression of running 5 ast passes instead of 2.
To complete this PR, I need to figure out "fix-point" and order of these ast passes.
close: #3969close: #2023
We need to add scope according to [this](d8086f14b6/src/compiler/binder.ts (L3883-L3962)). There are still some ASTs that need to be added to the scope.
---
Context from @Boshen:
Before this whole journey of fixing symbols and scopes I asked @Dunqing to debug through binder.ts via a debugger to fully understand how it does resolution.
We then agreed to align the implementation so that when a problem occurs, we can debug through both implementations and find where our problem is.
tsc doesn't have a specification, so we need to align with the reference implementation instead.
related: https://github.com/oxc-project/oxc/issues/4142#issuecomment-2219125356
Although we called `enter_node(Class)`, that doesn't mean we're in the `class` scope. It causes our must to visit decorators before `enter_node`.
Let's look at this case. It causes a syntax error if we don't visit decorators before `enter_node`
```js
// This file was procedurally generated from the following sources:
// - src/decorator/decorator-call-expr-identifier-reference-yield.case
// - src/decorator/syntax/valid/cls-expr-decorators-valid-syntax.template
/*---
description: Decorator @ DecoratorCallExpression (Valid syntax for decorator on class expression)
esid: prod-ClassExpression
features: [class, decorators]
flags: [generated, noStrict]
info: |
ClassExpression[Yield, Await] :
DecoratorList[?Yield, ?Await]opt class BindingIdentifier[?Yield, ?Await]opt ClassTail[?Yield, ?Await]
DecoratorList[Yield, Await] :
DecoratorList[?Yield, ?Await]opt Decorator[?Yield, ?Await]
Decorator[Yield, Await] :
@ DecoratorMemberExpression[?Yield, ?Await]
@ DecoratorParenthesizedExpression[?Yield, ?Await]
@ DecoratorCallExpression[?Yield, ?Await]
...
DecoratorCallExpression[Yield, Await] :
DecoratorMemberExpression[?Yield, ?Await] Arguments[?Yield, ?Await]
DecoratorMemberExpression[Yield, Await] :
IdentifierReference[?Yield, ?Await]
DecoratorMemberExpression[?Yield, ?Await] . IdentifierName
DecoratorMemberExpression[?Yield, ?Await] . PrivateIdentifier
IdentifierReference[Yield, Await] :
[~Yield] yield
...
---*/
function decorator() {
return () => {};
}
var yield = decorator;
var C = @yield() class {};
```
Errors:
```shell
× The keyword 'yield' is reserved
╭─[language/statements/class/decorator/syntax/valid/decorator-call-expr-identifier-reference-yield.js:45:2]
44 │
45 │ @yield() class C {}
· ─────
╰────
```
The changed code makes more sense. Only if we call `enter_scope` for class, the flags will contain `StrictMode`. Also, we can get the exact `flags` of the `scope` in the `class` at the transformer
For example:
```
class A {
B() {
// Before: flags is `Function`
// After: flags is `Function | StrictMode`
}
}
```
The regression tests will be fixed in follow-up PRs
```ts
class Klass <T> extends Root <R> {}
// ^^^^^ ^^^ ^^^^ ^^^ ^^
// id type_paramter super_class super_type_parameters body
```
I reorder fields according to the order above
The class scope is not defined in the spec. But we need to create a scope for `class` to store `TypeParamters`
I'm going to be AFK today(till about 9 PM UTC). Meanwhile, I Didn't want to be a blocker so here we go.
It would fix the #4200 merge if you guys find it in the correct order otherwise feel free to close it.
close: #4186
CatchClause has two scopes. The first one is `CatchClause`, which will add a `CatchParameter` to it. The second one is `Block`, which will add binding that declares in the current block scope.
The spec has a syntax error about `CatchParameter`
- It is a Syntax Error if any element of the BoundNames of CatchParameter also occurs in the LexicallyDeclaredNames of Block.
Adds checks to `TSImportEqualsDeclaration` for invalid use of `import type` modifier.
```ts
import { Foo } from './foo'
namespace Bar {
export class Baz {}
}
import type A = Foo.Baz; // not allowed
import type B = Bar.Baz; // not allowed
import type C = require('./c'); // allowed
```
