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

Use acorn for parsing JavaScript code #87

Merged
merged 31 commits into from
May 30, 2021
Merged

Use acorn for parsing JavaScript code #87

merged 31 commits into from
May 30, 2021

Conversation

TerrorJack
Copy link
Member

We used to use language-javascript for lexing or parsing JavaScript. This works for most cases, but the parser has rough edges (e.g. see #84 #86), and if we only do lexing, we don't have sufficient info about the AST (is it just an expression? is there top-level await?).

This PR switches to acorn for compile-time analysis of the inline JavaScript code in the quasiquoter. inline-js embeds a 108K bundled JS file, and use that for analysis. The source code for producing the bundle is also included.

The upsides of moving to acorn are:

  • Much better support of latest ECMAScript syntax features. acorn is a much more battle-tested JavaScript parser.
  • Much nicer API, one QuasiQuoter to rule them all. We used to have distinct expr/block quoters, then js/jsAsync. Now, we can finally only keep a single js quoter which supports all use cases.

The downside are:

  • Calls node at compile-time. Can be bad for a variety of reasons: bad for cross compilation; may be risky; may be slow.

Anyway, let's make the switch since this is something I've wanted to do for a long time.

Also sneaks in some other changes:

  • Use cabal for testing on Windows
  • Misc CI-related bikeshedding

Close #84 #86

@TerrorJack TerrorJack self-assigned this May 30, 2021
@TerrorJack TerrorJack merged commit 5da3690 into master May 30, 2021
@TerrorJack TerrorJack deleted the wip-acorn branch May 30, 2021 00:34
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.

HookToken and SpreadToken
1 participant