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

2 different SourceTypes needed for parser options, and for in AST #6249

Closed
overlookmotel opened this issue Oct 2, 2024 · 2 comments
Closed
Assignees
Labels
C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior

Comments

@overlookmotel
Copy link
Contributor

overlookmotel commented Oct 2, 2024

ModuleKind has 3 different options:

pub enum ModuleKind {
/// Regular JS script or CommonJS file
Script = 0,
/// ES6 Module
Module = 1,
/// Consider the file a "module" if ESM syntax is present, or else consider it a "script".
///
/// ESM syntax includes `import` statement, `export` statement and `import.meta`.
///
/// Note: Dynamic import expression is not ESM syntax.
///
/// See <https://babel.dev/docs/options#misc-options>
Unambiguous = 2,
}

The "unambiguous" option only makes sense as an input argument to the parser. The parser decides whether it's a script or module during parsing, so a Program is always one or the other, never "unambiguous".

Can we rule out the erroneous "unambiguous" option for Program::source_type by making the existing ModuleKind only "script" or "module", and introducing a separate SourceType to oxc_parser which is only used in ParseOptions?

While we're at it, we could also simplify SourceType by making it just a bitflags byte, or a 1-byte enum.

@overlookmotel overlookmotel added C-bug Category - Bug C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior and removed C-bug Category - Bug labels Oct 2, 2024
@Boshen
Copy link
Member

Boshen commented Oct 2, 2024

This is complicated ...

@Boshen Boshen self-assigned this Oct 2, 2024
@Boshen
Copy link
Member

Boshen commented Oct 10, 2024

While we're at it, we could also simplify SourceType by making it just a bitflags byte, or a 1-byte enum.

This is a huge breaking change because we export Language / ModuleKind / LanguageVariant, serialize them in a certain way.

Program::source_type vs ParserOptions::source_type

This introduces complexity to the project, where we need to distinguish the input and the output. We also need to implement conversions between them so they can be compared.


The amount of effort for making these changes is a bit of burden. I'll update the documentation instead around unambiguous instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category - technical debt or refactoring. Solution not expected to change behavior
Projects
None yet
Development

No branches or pull requests

2 participants