Conversation
…ated Migrates the hand-rolled Node.js interop bindings in `js_interop_gen` to 100% generated bindings, dogfooding the generator on itself. - Added `tool/node_entry.d.ts` declaring the curated Node.js APIs we use. - Added `tool/node_gen.yaml` configuration for generating Node bindings. - Added `tool/update_node_bindings.dart` runner script. - Replaced hand-rolled `fs` and `process` bindings in `lib/src/js/filesystem_api.dart` and `lib/src/js/node.dart` with the generated `lib/src/js/generated/node_api.dart`. - Defined safe options creator helpers in `filesystem_api.dart` to avoid calling non-existent JS constructors at runtime. - Refactored all internal callers of `fs` to use the new generated bindings and helpers, removing extensive manual `.toJS` and `.toDart` conversions for strings and booleans. - Updated integration test expectation `web_types_expected.dart` to match updated JSDoc comments from the new TypeScript package.
There was a problem hiding this comment.
Code Review
This pull request replaces manual Node.js interop definitions with generated bindings using a dogfooded configuration. It introduces a curated TypeScript definition file (node_entry.d.ts) and a script to generate the bindings, and updates the codebase to use these new APIs. Feedback suggests changing options bag declarations in the TypeScript file from class to interface to prevent the generator from producing problematic external constructors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces dogfooded, generated Node.js API bindings (node_api.dart) created from curated TypeScript definitions, replacing several hand-rolled bindings and simplifying file system operations across the codebase. The review feedback highlights two key areas for improvement: first, the generated extension types (fs, process, console) violate Dart's naming conventions by using lowercase and lack explicit @JS annotations, which makes them fragile; second, the generator should be updated to automatically emit external factory constructors for TypeScript interfaces, which would eliminate the need for hand-rolled helper functions like mkdirOptions.
…tors - Updated generator to emit UpperCamelCase extension type names (Fs, Process, Console) with explicit @js(...) namespace annotations for TypeScript namespaces. - Updated generator to automatically emit external factory constructors for TypeScript interface extension types. - Removed hand-rolled helper functions in favor of direct options bag constructors and direct imports. - Updated test expectation goldens.
|
Need to rebase this and drop the new Dart SDK type bits... |
Replace hand-rolled Node.js interop bindings (
filesystem_api.dart,node.dart) with bindings generated directly byjs_interop_genitself (lib/src/js/generated/node_api.dart).Summary of Changes
tool/update_node_bindings.dart,tool/node_gen.yaml, andtool/node_entry.d.tsto generatenode_api.dartusingjs_interop_genitself, replacing legacy hand-rolled binding files.js_interop_gen(dart_main.dart,util.dart,doc_provider.dart, etc.) to use the generatedFs,Process, and options bag interop types.@types/nodedependency topackage.json.