Skip to content

Fix exporting destructured variables#850

Merged
saghul merged 2 commits intomasterfrom
fix-parsing-import-destructure
Jan 24, 2025
Merged

Fix exporting destructured variables#850
saghul merged 2 commits intomasterfrom
fix-parsing-import-destructure

Conversation

@saghul
Copy link
Copy Markdown
Contributor

@saghul saghul commented Jan 24, 2025

@saghul saghul requested a review from bnoordhuis January 24, 2025 10:43
@saghul
Copy link
Copy Markdown
Contributor Author

saghul commented Jan 24, 2025

@bnoordhuis PTAL, this is my first foray into the parser 😅

Comment thread quickjs.c
static int js_parse_destructuring_element(JSParseState *s, int tok, int is_arg,
int hasval, int has_ellipsis,
bool allow_initializer)
bool allow_initializer, bool export_flag)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A suggestion for a follow-up PR: replace these 5 booleans with a flags argument. Call sites are IMO more legible as frob(HasArg|HasVal|HasEllipsis|AllowInitializer|ExportFlag) than frob(true,false,false,true,true)

(I suppose that, although its name suggests it's a boolean, has_ellipsis is actually a tri-state. The others are booleans though.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea, will do!

@saghul saghul merged commit f05a39a into master Jan 24, 2025
@saghul saghul deleted the fix-parsing-import-destructure branch January 24, 2025 21:44
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.

exporting destructured variables fails

2 participants