aglinxinyuan commented on code in PR #4997: URL: https://github.com/apache/texera/pull/4997#discussion_r3264007098
########## frontend/LICENSE-binary: ########## @@ -247,30 +248,40 @@ Angular / npm packages: - @ant-design/[email protected] - @auth0/[email protected] - @babel/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] - - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] + - @codingame/[email protected] Review Comment: The block at LICENSE-binary:279 is the @codingame/monaco-vscode-* service-override family. These are transitive deps of `monaco-vscode-api@25` (which `monaco-languageclient@10` depends on); each one is a separately-published facade that the codingame stack lazy-loads at editor init (`@codingame/monaco-vscode-files-service-override`, `@codingame/monaco-vscode-textmate-service-override`, etc.). We can't drop any individual one — the codingame entrypoint pulls each via `import("@codingame/monaco-vscode-*-service-override")` for the corresponding VS Code service, and `license-webpack-plugin` records everything webpack actually emits into a chunk. If we tried trimming, the next `yarn build:ci` would re-add them and `check_binary_deps.py` would flag a NEW-entry mismatch. All are MIT-licensed (same author / publisher), so the LICENSE-binary attribution is the simple group-by-license form. The list is generated, not hand-maintained. ########## frontend/LICENSE-binary: ########## @@ -283,18 +294,16 @@ Angular / npm packages: - [email protected] - [email protected] - [email protected] - - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - [email protected] - - [email protected] + - [email protected] Review Comment: We're not downgrading the direct dep — `package.json` still declares `marked: 17.0.1`. What `LICENSE-binary` reflects is the version actually bundled in `dist/`, and that's `[email protected]` — pulled in transitively by `[email protected]`, whose peer-pin still sits at the v14 line. `bin/licensing/check_binary_deps.py` aligns `LICENSE-binary` with `dist/3rdpartylicenses.json`, not with `package.json`. So this entry tracks what ships, not what we asked for. The day `ngx-markdown` bumps to `marked@17`, the listing will follow automatically. Confirmed via: ``` $ node -e "console.log(require('./dist/3rdpartylicenses.json').filter(p=>p.name==='marked').map(p=>p.name+'@'+p.version))" [ '[email protected]' ] ``` ########## frontend/package.json: ########## @@ -9,7 +9,6 @@ "start": "concurrently --kill-others \"npx y-websocket\" \"ng serve\"", "build": "node --max-old-space-size=8192 ./node_modules/@angular/cli/bin/ng build --configuration=production --progress=false --source-map=false", "build:ci": "node --max-old-space-size=8192 ./node_modules/nx/dist/bin/nx.js build --configuration=production --progress=false --source-map=false", - "analyze": "ng build --configuration=production --stats-json && webpack-bundle-analyzer dist/stats.json", Review Comment: The removed line was `"analyze": "ng build --configuration=production --stats-json && webpack-bundle-analyzer dist/stats.json"`. It pulled `[email protected]` in as a devDep purely to render a treemap of `dist/stats.json` locally. It was never invoked from CI and the dep itself wasn't used anywhere else — confirmed by `grep -r 'webpack-bundle-analyzer'` returning only the `package.json` declaration and the `analyze` script. Dropping both shrinks the install graph (~6 transitive deps). Happy to put both back if anyone uses the script locally — let me know. ########## frontend/vitest.config.ts: ########## @@ -29,17 +29,6 @@ export default defineConfig({ // which Angular's `fakeAsync` requires. Karma+Jasmine installed this // implicitly; the @angular/build:unit-test path doesn't. setupFiles: ["src/test-zone-setup.ts"], - // monaco-breakpoints' entry does `import './style.css'`. By default - // Vitest leaves third-party deps externalized, so Node's ESM loader - // tries to import the .css and crashes with - // `TypeError: Unknown file extension ".css"`. Inlining the package - // routes its imports through Vite/esbuild, which rewrites the CSS - // import to a no-op. - server: { - deps: { - inline: [/monaco-breakpoints/], - }, Review Comment: `[email protected]` is still in deps (top-level, line 53 of `package.json`) and `CodeDebuggerComponent` still drives it the same way it did pre-upgrade — `new MonacoBreakpoint(...)`, plus the line-marker decoration plumbing for left/right-click. The v10 upgrade didn't touch any of that surface; the package's editor API is unchanged. For test coverage there's `code-debugger.component.spec.ts` in the suite, which exercises construction + the workflow-status subscription path. You're right that we don't have a spec that actually clicks the gutter and asserts a breakpoint round-trips — that lives in the per-component scope (#4861 tracks pulling these out into Vitest browser mode where the editor can mount for real). For this PR my goal was "don't regress what's already covered"; happy to spin off a follow-up issue if you want the breakpoint UX pinned in tests. -- 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]
