-
-
Notifications
You must be signed in to change notification settings - Fork 386
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
feat: introduce precompiled-grammars #880
Conversation
✅ Deploy Preview for shiki-next ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for shiki-matsu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #880 +/- ##
==========================================
- Coverage 95.21% 95.17% -0.04%
==========================================
Files 83 86 +3
Lines 7063 7095 +32
Branches 1461 1468 +7
==========================================
+ Hits 6725 6753 +28
- Misses 330 334 +4
Partials 8 8 ☔ View full report in Codecov by Sentry. |
|
Although you're not currently using Also, if you bump to v0.10.0 as part of this PR, it's worth subsequently bumping |
if (value instanceof EmulatedRegExp) { | ||
return `new EmulatedRegExp(${JSON.stringify(value.rawArgs.pattern)},${JSON.stringify(value.rawArgs.flags)},${JSON.stringify(value.rawArgs.options)})` | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this code could somehow know whether the regex it's evaluating will have its captures used by the grammar, it could get a perf boost by avoiding the use of EmulatedRegExp
when capture values are not used (also described in #878).
if (value instanceof EmulatedRegExp) {
if (capturesUsedInGrammarForThisRegex || value.rawArgs.options.strategy) {
return `new EmulatedRegExp(${JSON.stringify(value.rawArgs.pattern)},${JSON.stringify(value.rawArgs.flags)},${JSON.stringify(value.rawArgs.options)})`
}
// For perf, avoid extra handling from `EmulatedRegExp` since it isn't needed in this case
return value.toString()
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you like to send a PR for that? Thanks!
resolves #878
Relies on:
@shikijs/themes
and@shikijs/langs
packages #879EmulatedRegExp
slevithan/oniguruma-to-es#15This PR introduces a new
@shikijs/langs-precompiled
package that inlines the compiled RegExp literals inside the grammar objects. And a new@shikijs/engine-javascript/raw
that only takes precompiled grammar to further optimize the performance and bundle size