Replace Bytecode with GremlinLang in JavaScript GLV by kirill-stepanishin · Pull Request #3307 · apache/tinkerpop

kirill-stepanishin · 2026-02-13T18:09:22Z

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 · 2026-02-13T18:22:54Z

Codecov Report

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

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3307      +/-   ##
============================================
- Coverage     77.87%   75.80%   -2.07%     
+ Complexity    13578    13271     -307     
============================================
  Files          1015     1020       +5     
  Lines         59308    59864     +556     
  Branches       6835     7034     +199     
============================================
- Hits          46184    45379     -805     
- Misses        10817    11815     +998     
- Partials       2307     2670     +363

☔ 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.

Cole-Greer

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.

Cole-Greer · 2026-02-13T20:31:25Z

...lin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.ts

+        'materializeProperties',
+        'bulkResults',
+      ];
+      for (const strategy of strategies) {


I expect all of the strategy handling here will need to be adjusted in the future. I think this is good enough for now and we can revisit this as we get more pieces of the driver functioning and get more tests running.

Cole-Greer · 2026-02-13T21:14:15Z

gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/graph-traversal.ts

+      return this.withStrategies(new OptionsStrategy({ [key]: val }));
    }
-    optionsStrategy[1].configuration[key] = val;
+    glStrategies[glStrategies.length - 1].configuration[key] = val;


I've got some questions around how OptionsStrategy needs to work as all the pieces of this driver come together. What you have here looks about right for now. It will become easier to reason about if any adjustments are needed here once more of the driver is complete.

Cole-Greer · 2026-02-13T21:19:59Z

gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/gremlin-lang.ts

+    if (arg === null || arg === undefined) return 'null';
+    if (typeof arg === 'boolean') return arg ? 'true' : 'false';
+    if (arg instanceof Long) {
+      return String(arg.value);


I think we should probably append an L suffix on longs.

Cole-Greer · 2026-02-13T21:22:07Z

gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/gremlin-lang.ts

+      return String(arg.value);
+    }
+    if (arg instanceof Date) {
+      const iso = arg.toISOString().replace('.000Z', 'Z');


Was this added to be more consistent with Java, Python, and Go? I think it's ok to save a step here and keep .000Z as both are equivalent. Not sure if anyone has other opinions on this.

I checked Java, Python, and Go, all three omit .000 for zero-millisecond dates. I did it for consistency, but I would be happy to remove it if that makes more sense

I slightly prefer removal but that's not a strong preference. Given the lack of other opinions, I'm happy to defer to you as the PR author for this one.

Cole-Greer · 2026-02-13T21:26:04Z

gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/gremlin-lang.ts

+      return String(arg);
+    }
+    if (typeof arg === 'string') {
+      const escaped = JSON.stringify(arg).slice(1, -1).replace(/'/g, "\\'");


I'm not understanding what the JSON.stringify() and slice() are doing here. Could we just do a replace() directly on arg itself?

JSON.stringify() escapes all special characters in one call, but wraps the result in double quotes and doesn't escape single quotes.
So .slice(1, -1) to strip the double quotes, .replace() to escape single quotes, then wrap in single quotes for Gremlin.
I think it also could either be done by nesting .replace() for every case or with a single .replace() + regex + replacer function, but it'd be more verbose and require manually maintaining the escape mapping.
I'll add comment to clarify.

This is reminding me that our argument escaping is somewhat inconsistent across GLVs, we really standardize and document recommendations for escaping gremlin-lang scripts. Spinning this out into its own JIRA: https://issues.apache.org/jira/browse/TINKERPOP-3233

Cole-Greer · 2026-02-13T22:24:06Z

gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/gremlin-lang-test.js

+      // #91
+      [g.withStrategies(new SubgraphStrategy({vertices: __.has('region','US-TX'), vertexProperties: __.hasNot('runways')})).V().count(), "g.withStrategies(new SubgraphStrategy(vertices:__.has('region','US-TX'),vertexProperties:__.hasNot('runways'))).V().count()"],
+      // #92
+      [g.withStrategies(new ReadOnlyStrategy(), new SubgraphStrategy({vertices: __.has('region','US-TX'), edges: __.hasLabel('route')})).V().count(), "g.withStrategies(ReadOnlyStrategy,new SubgraphStrategy(vertices:__.has('region','US-TX'),edges:__.hasLabel('route'))).V().count()"],


I'm noticing the new keyword is only used in the GremlinLang if the strategy has arguments (ReadOnlyStrategy does not use new in this case, but SubgraphStrategy does). That appears to be consistent with Java but it is odd. I think that's fine for now but might be worth a revisit later.

Cole-Greer · 2026-02-13T22:26:46Z

gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/gremlin-lang-test.js

+
+  describe('JS-specific tests', function () {
+    it('should handle Long values', function () {
+      assert.strictEqual(g.V(new Long('9007199254740993')).getGremlinLang().getGremlin(), 'g.V(9007199254740993)');


I think this should result in 'g.V(9007199254740993L)'

Cole-Greer · 2026-02-13T22:35:10Z

gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/traversal-test.js

-      assert.strictEqual(bytecode.stepInstructions[2][2].elementName, 'desc');
-    });
-
-    it('should add steps with Direction aliases from_ and to properly mapped to OUT and IN', function () {


I think these test cases are still valuable, although they should just assert the final GremlinLang string matches the expected result instead of all of these assertions for the bytecode. The first 2 and final cases all appear to be adequately covered by gremlin-lang-test already, but this case is not. Could you restore the test here or add this case to gremlin-lang-test?

Cole-Greer · 2026-02-13T22:37:49Z

gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/traversal-test.js

-      assert.strictEqual(vStartBytecode.stepInstructions[0][2], 2); // ID should be extracted from Vertex
-
-      // Test V() step in the middle of a traversal
-      const vMid = g.inject('foo').V(1, new Vertex(2, 'person'));


Similar to my above comment, I don't see an equivalent mid-traversal V() case in gremlin-lang-test

Cole-Greer · 2026-02-13T22:40:01Z

gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/traversal-test.js

-      assert.strictEqual(fromToBytecode.stepInstructions[2][0], 'to');
-      assert.strictEqual(fromToBytecode.stepInstructions[2][1], 2); // ID should be extracted from Vertex
-    });
-    it('should extract ID from Vertex objects for mergeE()', function () {


Similar to above, I don't see an equivalent for this in gremlin-lang-test.

kenhuuu · 2026-02-17T20:42:29Z

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.

xiazcy · 2026-02-17T21:01:12Z

gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/gremlin-lang.ts

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


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?

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 · 2026-02-17T23:09:44Z

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

Replace Bytecode with GremlinLang

f452656

Cole-Greer reviewed Feb 13, 2026

View reviewed changes

xiazcy reviewed Feb 17, 2026

View reviewed changes

Conversation

kirill-stepanishin commented Feb 13, 2026

Changes

Intentionally out of scope for this PR

Uh oh!

codecov-commenter commented Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Cole-Greer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Cole-Greer Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kenhuuu commented Feb 17, 2026

Uh oh!

xiazcy Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xiazcy commented Feb 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Feb 13, 2026 •

edited

Loading

Cole-Greer Feb 17, 2026 •

edited

Loading

xiazcy Feb 17, 2026 •

edited

Loading