Stabilize JS Interop CI and decouple webref dependencies#532
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the interop_generator by moving Web IDL-specific generation logic into a new web_generator package and introducing a JSON-based data flow for IDL, CSS, and browser compatibility data. This change decouples the core generator from direct dependencies on Node.js packages like @webref. Feedback focuses on improving the robustness of the implementation by addressing unsafe type casts during library analysis, potential type errors when accessing JSON maps, and a possible StateError in a list manipulation utility.
- Renamed package from `interop_generator` to `js_interop_gen` to align with the monorepo structure. - Removed `@webref` dependencies from `js_interop_gen/lib/src/package.json` to prevent implicit runtime reliance on the external `@webref` ecosystem. - Refactored `generateBindingsForFiles` in `generate_bindings.dart` to pass an empty JS object instead of relying on missing global JS state for CSS and element definitions, resolving runtime crashes when dependencies were removed. - Updated `idl_test.dart` to use the real BCD file from `web_generator` to satisfy the parser and keep tests passing, while adding a TODO to track the removal of this cross-package dependency in the future. - Fixed analyzer issues in `generate_bindings.dart` related to type inference on collection literals and line length limits. - Added GitHub CI status badges to the root `README.md` for all packages in the repository, removing the stale TODO comment.
5463a6c to
c01f30c
Compare
|
I think this is ready for review @srujzs CC @nikeokoronkwo and @nex3 |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the project by extracting the core binding generation logic into a new package, js_interop_gen, and specializing web_generator for package:web. It introduces CLI options for pre-parsed IDL and browser-compat-data JSON files, adds support for async_iterable, and moves MDN conversion logic to a dedicated library. Feedback was provided to correct typos in the js_interop_gen documentation.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
I agree with Daco here that spelling it out is better and is consistent with |
|
@srujzs – so you're saying leave it as is in the PR? |
| // for details. All rights reserved. Use of this source code is governed by a | ||
| // BSD-style license that can be found in the LICENSE file. | ||
|
|
||
| import 'dart:io'; |
There was a problem hiding this comment.
Moving this OUT of the core logic. Running the generator shouldn't update the generator!!
The code does yell if it needs updating, though!
| @@ -1,4 +1,3 @@ | |||
| [](https://github.com/dart-lang/web/actions/workflows/web.yaml) | |||
There was a problem hiding this comment.
No need to advertise the CURRENT CI status on the package site!
| return null; | ||
| } | ||
|
|
||
| String convertMdnToMarkdown(String content) { |
There was a problem hiding this comment.
This was moved to a library so we could unit test it!
| import 'package:test/test.dart'; | ||
|
|
||
| import '../bin/scrape_mdn.dart'; | ||
| import 'package:web_generator/src/mdn_converter.dart'; |
There was a problem hiding this comment.
importing things from bin in tests - not good!
| web_generator/lib/src/*.js | ||
| web_generator/lib/src/*.js.* | ||
| web_generator/lib/src/node_modules/ | ||
| js_interop_gen/lib/src/*.js |
There was a problem hiding this comment.
If we ever want to PUBLISH this package, we'll need to include the JS bits so it works outside of the repo!
That's my preference, yes. |
LOL – I guess it was your initial suggestion! 😆 |
| run on Node. | ||
|
|
||
| There is one entrypoint present in this package: | ||
| - `js_interop_gen.dart`: This entrypoint is for generating Dart interfaces from TS Declaration code, given the path to a `.d.ts` file |
There was a problem hiding this comment.
*or an IDL file (it seems this package still works with IDL code)
srujzs
left a comment
There was a problem hiding this comment.
A step closer into full disentanglement, yay!
- Centralize generated assets (web_apis.json, dummy_bcd.json) into package-specific subdirectories under .dart_tool. - Remove hardcoded web_apis.json and dummy_bcd.json from the repository. - Update update_idl_bindings.dart to automatically run preparse_idls.mjs when needed. - Make JSON casts and lookups in bcd.dart more robust to avoid TypeErrors with minimal data. - Remove redundant npm install step for web_generator in js_interop_gen.yaml workflow. - Update tests to use the new ignored paths.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the repository by extracting general-purpose JS interop generation logic into a new package, js_interop_gen, while repurposing web_generator for internal package:web specific tasks. The changes involve moving core AST, translation, and CLI logic, updating dependencies, and refactoring data fetching to support external JSON paths for IDL and browser compatibility data. Feedback focuses on improving the robustness of the generated file check by normalizing line endings to prevent false positives on Windows and ensuring consistent command-line instructions in warning messages and file headers.
|
...like...approve? |
…, webdev, webdriver Revisions updated by `dart tools/rev_sdk_deps.dart`. ai (https://github.com/dart-lang/ai/compare/3d3b7fd..7270c43): 7270c43 Wed Apr 29 01:14:51 2026 +0530 Sourav-Sonkar fix: graceful shutdown of orphaned test apps on windows (dart-lang/ai#438) 07af327 Tue Apr 28 11:11:31 2026 -0700 Jacob MacDonald Update dependencies (dart-lang/ai#453) ff1866d Wed Apr 29 00:13:33 2026 +0800 Goweii fix: Add empty properties object to tool input schemas to resolve Claude 400 validation error (dart-lang/ai#444) 8ce7450 Thu Apr 16 11:03:55 2026 -0700 Jacob MacDonald add gemini code assist configuration for reviews (dart-lang/ai#443) 899e562 Tue Apr 14 11:38:29 2026 -0700 Jacob MacDonald Merge cherry-pick branch into main (dart-lang/ai#442) 5eae0ae Tue Apr 14 18:27:49 2026 +0000 Jake Macdonald Merge remote-tracking branch 'origin/cherry-pick' into merge-cherry-pick 1e69aa8 Tue Apr 14 11:20:43 2026 -0700 Jacob MacDonald re-implement most of `#440` for the current stable release (dart-lang/ai#441) core (https://github.com/dart-lang/core/compare/347df4b..be0b1531): be0b1531 Wed Apr 29 16:50:55 2026 -0700 Nate Bosch Revert "Completely new cross-platform API introduced" (#957) a23519da Mon Apr 20 15:57:58 2026 -0700 Nate Bosch Support variable duration in RestartableTimer (dart-lang/core#949) 0def20c4 Thu Apr 2 08:40:37 2026 +0200 Lasse R.H. Nielsen Completely new cross-platform API introduced dartdoc (https://github.com/dart-lang/dartdoc/compare/c01cf53..2e30b8e): 2e30b8e3 Tue Apr 21 15:09:21 2026 -0700 Kevin Moore Improve @canonicalFor validation with close-match suggestions (dart-lang/dartdoc#4234) fbc326ea Tue Apr 14 09:18:26 2026 -0700 Kevin Moore Fix dartdoc canonicalization for explicit getters and prioritize @canonicalFor (dart-lang/dartdoc#4233) ecosystem (https://github.com/dart-lang/ecosystem/compare/3bd43d6..ed4e053): ed4e053 Wed Apr 15 08:30:59 2026 -0700 Kevin Moore fix(workflows): prevent duplicate PR comments in post_summaries.yaml (dart-lang/ecosystem#412) http (https://github.com/dart-lang/http/compare/fa2d2c2..2c84f24): 2c84f24 Mon Apr 27 14:45:17 2026 -0700 Brian Quinlan doc: s/dart:html/package:web/g (dart-lang/http#1911) i18n (https://github.com/dart-lang/i18n/compare/de7e11b..2ae32fd): 2ae32fdd Wed Apr 22 02:20:51 2026 -0700 Googler No public description 1eaf8a2e Sun Mar 1 06:51:04 2026 +0000 dependabot[bot] Bump the github-actions group with 2 updates (dart-lang/i18n#1053) shelf (https://github.com/dart-lang/shelf/compare/c4b94d3..cc6b57d): cc6b57d Wed Apr 29 08:59:40 2026 -0700 Kevin Moore chore: update shelf_io test to not crash on certificate error (dart-lang/shelf#522) da9f1b1 Thu Apr 23 14:43:00 2026 -0400 Mouad Debbar Fix multi-header handling in shelf_proxy (dart-lang/shelf#519) a6693a9 Wed Apr 22 23:15:09 2026 -0700 Kevin Moore feat: Add HTTP/1.1 compliance testing package and baseline for shelf (dart-lang/shelf#518) test (https://github.com/dart-lang/test/compare/8bbb847..d5da922): d5da9229 Mon Apr 27 17:32:33 2026 -0700 Jacob MacDonald Expose the backend APIs used by flutter_test from test_api (dart-lang/test#2635) c8c76f45 Mon Apr 27 13:38:25 2026 -0700 Nate Bosch Prepare to publish (dart-lang/test#2634) web (https://github.com/dart-lang/web/compare/15599ee..7c908b1): 7c908b1 Tue Apr 28 17:16:57 2026 -0700 Kevin Moore test(js_interop_gen): add integration tests (dart-lang/web#541) 5ed9bed Tue Apr 28 17:04:41 2026 -0700 Kevin Moore js_interop_gen: minimize the number of `@docImport` comments we create (dart-lang/web#540) 2c47e36 Mon Apr 27 20:55:43 2026 -0700 Kevin Moore feat: generate descriptive documentation for union typedefs (dart-lang/web#534) db91caf Mon Apr 27 15:15:30 2026 -0700 Kevin Moore chore: write BCD version to README in web_generator (dart-lang/web#537) 5e7285f Fri Apr 24 12:09:10 2026 -0700 Kevin Moore chore: update node invocation to use the source maps we already generate (dart-lang/web#538) 3a7e37a Sat Apr 18 19:31:20 2026 -0700 Kevin Moore Stabilize JS Interop CI and decouple webref dependencies (dart-lang/web#532) d7daf7d Tue Apr 14 12:23:21 2026 -0700 Kevin Moore generator: add some sanity to the language version logic (dart-lang/web#530) b026317 Mon Apr 13 13:01:37 2026 -0700 Kevin Moore generator: be more robust against NPM timing issues (dart-lang/web#528) webdev (https://github.com/dart-lang/webdev/compare/2947c78..d33d270): d33d2704 Thu Apr 23 16:00:22 2026 -0400 Jessy Yameogo [dwds] Sync state with sdk/g3 and restore static analysis for fixtures (dart-lang/webdev#2824) e43744b2 Fri Apr 17 16:55:24 2026 -0400 Jessy Yameogo [dwds] Fix ping event serialization by introducing PingRequest (dart-lang/webdev#2821) 1063eb00 Fri Apr 17 10:28:47 2026 -0700 MarkZ Creating experimental flag for web hot reload in webdev and splitting tests between DDC module systems (dart-lang/webdev#2807) webdriver (https://github.com/google/webdriver.dart/compare/26326d3..3a711eb): 3a711eb Mon Apr 27 10:09:31 2026 -0700 Kevin Moore Update CI badge to point to master branch (google/webdriver.dart#343) R=bquinlan@google.com Change-Id: Ie746dfd8f9637db46075b252d3290925de11b382 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/499663 Reviewed-by: Brian Quinlan <bquinlan@google.com> Commit-Queue: Nate Bosch <nbosch@google.com>
js_interop_genpackage that doesn't know aboutpkg:web@webrefdependencies fromjs_interop_gen/lib/src/package.jsonto prevent implicit runtime reliance on the external@webrefecosystem.generateBindingsForFilesingenerate_bindings.dartto pass an empty JS object instead of relying on missing global JS state for CSS and element definitions, resolving runtime crashes when dependencies were removed.idl_test.dartto use the real BCD file fromweb_generatorto satisfy the parser and keep tests passing, while adding a TODO to track the removal of this cross-package dependency in the future.generate_bindings.dartrelated to type inference on collection literals and line length limits.README.mdfor all packages in the repository, removing the stale TODO comment.