Skip to content

Replace Bytecode with GremlinLang in JavaScript GLV#3307

Merged
Cole-Greer merged 2 commits intoapache:masterfrom
kirill-stepanishin:bytecode-to-gremlinlang-js
Feb 18, 2026
Merged

Replace Bytecode with GremlinLang in JavaScript GLV#3307
Cole-Greer merged 2 commits intoapache:masterfrom
kirill-stepanishin:bytecode-to-gremlinlang-js

Conversation

@kirill-stepanishin
Copy link
Contributor

As part of the TP4 migration, the server no longer accepts Bytecode and only processes Gremlin queries in script form.. This PR replaces Bytecode + Translator with a new GremlinLang class.

Changes

  • New GremlinLang class that incrementally builds Gremlin query strings as steps are added
  • GraphTraversalSource / GraphTraversal now accumulate into GremlinLang instead of Bytecode
  • Client.submit() only accepts strings now; all queries sent as gremlin-lang language. OptionsStrategy extracted into RequestOptions (timeout, batchSize, bulkResults, etc.)
  • Deleted Bytecode, Translator, BytecodeSerializer, and their tests
  • Removed Bytecode serializers from both GraphSON and GraphBinary layers

Intentionally out of scope for this PR

  • UUID — no UUID("...") wrapper; UUIDs fall through to String(arg)
  • Numeric type suffixes (1B, 2S, 4L, 5.0F, 6.0D, etc.)

@codecov-commenter
Copy link

codecov-commenter commented Feb 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.82%. Comparing base (cfd6889) to head (ee834e1).
⚠️ Report is 791 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3307      +/-   ##
============================================
- Coverage     77.87%   75.82%   -2.05%     
+ Complexity    13578    13276     -302     
============================================
  Files          1015     1020       +5     
  Lines         59308    59864     +556     
  Branches       6835     7034     +199     
============================================
- Hits          46184    45391     -793     
- Misses        10817    11804     +987     
- Partials       2307     2669     +362     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@Cole-Greer Cole-Greer left a comment

Choose a reason for hiding this comment

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

Overall I think this looks really good. Most of my comments are just for context and not anything which needs action in this PR, the rest are all for minor clarifications and tweaks.

@kenhuuu
Copy link
Contributor

kenhuuu commented Feb 17, 2026

The overall design is similar to what has been done to the other GLVs for GremlinLang so my VOTE is +1, pending resolution of other comments.

const iso = arg.toISOString().replace('.000Z', 'Z');
return `datetime("${iso}")`;
}
if (typeof arg === 'number') {
Copy link
Contributor

@xiazcy xiazcy Feb 17, 2026

Choose a reason for hiding this comment

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

Given JS doesn't distinguish the number types, I wonder if we should be identifying the numerical types in gremlin by asserting their sizes for adding the suffix instead of just turning it to string directly?

Copy link
Contributor

Choose a reason for hiding this comment

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

Numbers will be a giant mess here as there's no clear way to assume the users intent and map number into all of the TP numeric types. I expect we will need to accept some amount of precision loss and/or non-roundtripable types. My vote is to leave this for now and handle it separately. That topic alone can easily spawn many debates around handling subtle edge cases.

@xiazcy
Copy link
Contributor

xiazcy commented Feb 17, 2026

Overall I think this is a solid start for switching to GremlinLang in JS. There will be areas that needs follow-up changes, but I'm good to leave them out of this PR. VOTE +1

@Cole-Greer
Copy link
Contributor

Thanks for the changes @kirill-stepanishin. LGTM

VOTE +1

@Cole-Greer Cole-Greer merged commit b6ad7fe into apache:master Feb 18, 2026
21 checks passed
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.

5 participants