Skip to content

fix: detect every binding in export var/let/const declarations - #199

Merged
guybedford merged 2 commits into
guybedford:mainfrom
BridgeAR:BridgeAR/2026-06-16-export-binding-detection
Jun 28, 2026
Merged

fix: detect every binding in export var/let/const declarations#199
guybedford merged 2 commits into
guybedford:mainfrom
BridgeAR:BridgeAR/2026-06-16-export-binding-detection

Conversation

@BridgeAR

@BridgeAR BridgeAR commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

export var/let/const declarations reported only their first binding. The declaration scanner stopped at the first = initializer, so every later declarator was dropped (export const a = 1, b = 2 reported only a), and the destructuring path reported the property key rather than the bound name (export const { a: b } = x reported a), missing defaults, nested patterns and rest elements.

The scanner now walks the whole binding list: it reads each bound name and skips every initializer and default value expression-aware, so a comma inside one no longer ends the list, and the list terminates at ;, EOF or an ASI line break (never reading the next statement as a binding).

That expression skip shares the main tokenizer (a single consumeToken), so its regex / keyword / import() resolution cannot diverge from the main scan.

Fixes #176
Fixes #175

@BridgeAR
BridgeAR force-pushed the BridgeAR/2026-06-16-export-binding-detection branch from 58694b5 to 94d3cf4 Compare June 16, 2026 12:12
@BridgeAR
BridgeAR marked this pull request as ready for review June 16, 2026 12:48
@guybedford

guybedford commented Jun 21, 2026

Copy link
Copy Markdown
Owner

Really nice to finally get every binding name out of these declarations, and the destructuring walk in readBindingPattern/readBindingTarget is clean — that part is genuinely new work and reads well.

My one substantive concern is skipExpression. It re-implements regex-vs-division disambiguation from scratch (isValueChar + lastWasValue), and that's a weaker copy of logic the lexer already does correctly in the main loop. Because it only looks at the previous character rather than the previous token, it misses the value-keyword case, and on valid input that now throws rather than under-reporting:

export const f = () => { return /,/g }, b = 2;   // → throws "Parse error"

return ends in a value char, so the / is read as division; the regex's closing / then opens a second, phantom regex that runs to EOF. On main this parsed fine (it bailed at the first =), so this is a regression — and throwing is strictly worse than the old under-reporting, since it makes the whole module un-analyzable.

The deeper point is that skipExpression shouldn't own this decision at all. The main tokenizer already tracks everything needed and resolves the ambiguity properly at src/lexer.c:204-238, keyed off real lexer state:

  • lastTokenPos — the previous token (so isExpressionKeyword catches return/typeof/in/etc., isExpressionPunctuator catches operators, isParenKeyword catches if (...) /re/)
  • openTokenStack / openTokenDepth — nesting, including which token preceded each opening bracket (ClassBrace, isExpressionTerminator, the for/of binding case)
  • lastSlashWasDivision — the sticky flag for / chains

skipExpression re-derives a thin version of all of this (its local depth shadows openTokenStack, its lastWasValue shadows the :204 decision) and also re-dispatches strings/templates/comments that the main loop already routes through stringLiteral/templateString/lineComment/blockComment. So we have two divergent copies of a subtle invariant, and the new one is the buggy one.

Two ways to share the real machinery, in order of preference:

1. Ride the main loop. The only thing the binding scan genuinely needs that the tokenizer doesn't already give us is "is this ,/; the end of the binding list?" — which is just openTokenDepth == <depth recorded at the declaration>. If we extract the per-token body of the mainparse switch into a consumeToken() that updates the globals, then both the main loop and the initializer-skip call it, and the skip just adds the terminator check after each token. Regex, strings, templates, comments, and lastSlashWasDivision all stay correct by construction because it is the tokenizer.

2. Minimal: factor just the decision. Pull :204-238 into a bool slashIsRegex() and have skipExpression use the global openTokenStack/openTokenDepth/lastTokenPos instead of its private depth/lastWasValue. Smaller diff, kills the divergent regex copy and the throw, but skipExpression still re-dispatches strings/templates/comments — so less clean than (1).

Either way the return /,/g case goes away for free, and we stop maintaining two copies of the regex heuristic.

The genuinely new logic that has to stay in the export path regardless: ASI termination (line break after a value), since the main loop doesn't care about statement boundaries, and the binding-target/pattern reading. Both are fine as-is.

@BridgeAR
BridgeAR force-pushed the BridgeAR/2026-06-16-export-binding-detection branch from 94d3cf4 to 4c9faa4 Compare June 21, 2026 22:21
@BridgeAR
BridgeAR marked this pull request as draft June 22, 2026 00:37
@BridgeAR
BridgeAR force-pushed the BridgeAR/2026-06-16-export-binding-detection branch from 4c9faa4 to e21a484 Compare June 22, 2026 10:55
@BridgeAR
BridgeAR marked this pull request as ready for review June 22, 2026 11:26
@BridgeAR

Copy link
Copy Markdown
Collaborator Author

I updated the code to use as much of the main loop as possible. I tried to make sure the performance stays the same while being able to parse more and that was the reason I pulled out consumeToken.

BridgeAR and others added 2 commits June 28, 2026 14:08
A multi-declarator export reported only the first binding:
`export const a = 1, b = 2` yielded `a`, never `b` - the initializer scanner
stopped at the first declarator instead of skipping each initializer to its
separating comma.

skipExpression now rides consumeToken, the one tokenizer the main loop uses, so
its regex, keyword and import resolution cannot diverge from the main scan. That
also fixes four cases a separate, simplified scanner got wrong:

1. A '/' after a value keyword (return/typeof/yield/void) opens a regex, so the
   comma in `export const f = () => { return /,/g }, b = 2` no longer truncates
   the list.
2. A '}' closing a block leaves the next '/' in statement position (a regex).
3. import() in an initializer or a destructuring default is detected.
4. A numeric or BigInt property key such as `{ 0.5: b }` is consumed whole.
@guybedford
guybedford force-pushed the BridgeAR/2026-06-16-export-binding-detection branch from a5e5a16 to 9c25cc0 Compare June 28, 2026 22:15
@guybedford
guybedford enabled auto-merge (squash) June 28, 2026 22:16
@guybedford
guybedford merged commit 064d0db into guybedford:main Jun 28, 2026
1 check passed
mergify Bot added a commit to ArcadeData/arcadedb that referenced this pull request Jul 5, 2026
…2.3.0 in /studio [skip ci]

Bumps [es-module-lexer](https://github.com/guybedford/es-module-lexer) from 2.1.0 to 2.3.0.
Release notes

*Sourced from [es-module-lexer's releases](https://github.com/guybedford/es-module-lexer/releases).*

> 2.2.0
> -----
>
> What's Changed
> --------------
>
> * build(deps-dev): update dev dependencies to latest by [`@​BridgeAR`](https://github.com/BridgeAR) in [guybedford/es-module-lexer#200](https://redirect.github.com/guybedford/es-module-lexer/pull/200)
> * fix: report n for no-substitution template dynamic imports by [`@​BridgeAR`](https://github.com/BridgeAR) in [guybedford/es-module-lexer#201](https://redirect.github.com/guybedford/es-module-lexer/pull/201)
> * Automatic asm.js dictionary extraction and clang toolchain update by [`@​guybedford`](https://github.com/guybedford) in [guybedford/es-module-lexer#203](https://redirect.github.com/guybedford/es-module-lexer/pull/203)
> * fix: don't treat a method named `import` with 2+ args as a dynamic import by [`@​soberm`](https://github.com/soberm) in [guybedford/es-module-lexer#207](https://redirect.github.com/guybedford/es-module-lexer/pull/207)
> * fix: detect every binding in export var/let/const declarations by [`@​BridgeAR`](https://github.com/BridgeAR) in [guybedford/es-module-lexer#199](https://redirect.github.com/guybedford/es-module-lexer/pull/199)
> * feat: expose export statement start position as ss by [`@​BridgeAR`](https://github.com/BridgeAR) in [guybedford/es-module-lexer#206](https://redirect.github.com/guybedford/es-module-lexer/pull/206)
>
> New Contributors
> ----------------
>
> * [`@​BridgeAR`](https://github.com/BridgeAR) made their first contribution in [guybedford/es-module-lexer#200](https://redirect.github.com/guybedford/es-module-lexer/pull/200)
> * [`@​soberm`](https://github.com/soberm) made their first contribution in [guybedford/es-module-lexer#207](https://redirect.github.com/guybedford/es-module-lexer/pull/207)
>
> **Full Changelog**: <guybedford/es-module-lexer@2.1.0...2.2.0>


Commits

* [`dbac1c3`](guybedford/es-module-lexer@dbac1c3) 2.3.0
* [`f3c972d`](guybedford/es-module-lexer@f3c972d) fix: allow wasm memory growth in the minimal build, document it in the readme...
* [`e9b611e`](guybedford/es-module-lexer@e9b611e) fix: allow wasm memory growth for sources over ~4MB ([#217](https://redirect.github.com/guybedford/es-module-lexer/issues/217))
* [`6c90425`](guybedford/es-module-lexer@6c90425) feat: add minimal build for es-module-shims (es-module-lexer/minimal) ([#211](https://redirect.github.com/guybedford/es-module-lexer/issues/211))
* [`70010fb`](guybedford/es-module-lexer@70010fb) docs: correct stale limitations and gzip size in README ([#212](https://redirect.github.com/guybedford/es-module-lexer/issues/212))
* [`1f19494`](guybedford/es-module-lexer@1f19494) 2.2.0
* [`d61e97a`](guybedford/es-module-lexer@d61e97a) feat: expose export statement start position as ss ([#206](https://redirect.github.com/guybedford/es-module-lexer/issues/206))
* [`064d0db`](guybedford/es-module-lexer@064d0db) fix: detect every binding in export var/let/const declarations ([#199](https://redirect.github.com/guybedford/es-module-lexer/issues/199))
* [`99b5226`](guybedford/es-module-lexer@99b5226) fix: don't treat a method named `import` with 2+ args as a dynamic import ([#207](https://redirect.github.com/guybedford/es-module-lexer/issues/207))
* [`fa1ade6`](guybedford/es-module-lexer@fa1ade6) build: update to upstream emsdk toolchain and derive asm.js dictionary from t...
* Additional commits viewable in [compare view](guybedford/es-module-lexer@2.1.0...2.3.0)
  
[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility\_score?dependency-name=es-module-lexer&package-manager=npm\_and\_yarn&previous-version=2.1.0&new-version=2.3.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
Dependabot commands and options
  
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot show  ignore conditions` will show all of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
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.

Support multiple exports per declaration Support renamed destructured exports

2 participants