Cole-Greer commented on code in PR #3382: URL: https://github.com/apache/tinkerpop/pull/3382#discussion_r3096526371
########## docs/gremlator/vite.config.ts: ########## @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/// <reference types="vitest" /> +import { defineConfig } from 'vite'; +import react from '@vitejs/plugin-react'; + +export default defineConfig({ + plugins: [react()], + base: '/gremlator', Review Comment: ```suggestion base: '/gremlator', esbuild: { keepNames: true, }, ``` In local testing, I was noticing that step arguments seemed to just pass straight through without any translation. <img width="942" height="396" alt="image" src="https://github.com/user-attachments/assets/3df9df97-42d5-447c-856a-60731fc306aa"; /> <img width="895" height="386" alt="image" src="https://github.com/user-attachments/assets/f75b38ad-13b2-41fa-9134-47d4794b6e42"; /> My AI overlord of choice had this to say about it, resulting in this fix: > The TranslateVisitor.visit() method dispatches to the correct visitor method (e.g. visitStringLiteral, visitIntegerLiteral) by inspecting tree.constructor.name at runtime — it expects names like StringLiteralContext and strips the Context suffix to find the method. > > Vite's production build minifies class names, so StringLiteralContext becomes something like Qh. The className.endsWith('Context') check fails, the dispatch never matches any specific visitor method, and every rule context falls through to the generic visitChildren() path, which just walks down to terminal tokens and outputs raw text. > > That's why step names translate correctly (they're terminal tokens handled by visitTerminal) but all arguments — strings, numbers, booleans, collections — pass through unmodified. The language-specific visitor overrides for those literal types never get called. > > The fix is adding esbuild: { keepNames: true } to vite.config.ts so class names survive minification. But the deeper issue is in gremlin-js itself: the visit() dispatch relies on runtime class name reflection, which is inherently fragile under minification. A more robust approach would be to dispatch on ruleIndex (a stable integer) rather than the class name string. That is quite an unexpected failure mode in my opinion. It does give me some concerns about the frailty of the JS translators in embedded webapp deployments (which is their intended purpose). I don't think the translator fragility is of concern for this PR, but it's something to think about going forward. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
