From e103caf4bfd9c37e62b1d8622094e45ce73edbc5 Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Tue, 3 Sep 2024 15:46:25 -0700 Subject: [PATCH 1/2] Add a generate-all flag to emit all bindings In the case where a user wants to use an experimental API that we don't include in package:web, it is useful to have the generator emit bindings still so they can copy it over instead of having to write all of it themselves. This also allows us to find problems in our tools with newer bindings before they become standard. - Adds a generate-all flag to the tools and refactors code to incorporate it when determining whether to generate declarations. - Cleans up argument passing so we're always using package:args. - Adds a generate_all_and_analyze to the Dart CI. --- .github/workflows/dart.yml | 86 ++++++++++++++++++++ mono_repo.yaml | 1 + tool/ci.sh | 4 + web_generator/CHANGELOG.md | 8 +- web_generator/README.md | 11 +++ web_generator/bin/update_bindings.dart | 13 ++- web_generator/lib/src/bcd.dart | 16 +++- web_generator/lib/src/dart_main.dart | 25 ++++-- web_generator/lib/src/generate_bindings.dart | 6 +- web_generator/lib/src/main.mjs | 4 +- web_generator/lib/src/translator.dart | 28 ++++--- web_generator/mono_pkg.yaml | 3 + 12 files changed, 173 insertions(+), 32 deletions(-) diff --git a/.github/workflows/dart.yml b/.github/workflows/dart.yml index 03caf814..8e490c5c 100644 --- a/.github/workflows/dart.yml +++ b/.github/workflows/dart.yml @@ -420,6 +420,90 @@ jobs: - job_008 - job_009 job_012: + name: "generate_all_and_analyze; Dart dev; PKG: web_generator; `dart analyze --fatal-infos .`" + runs-on: ubuntu-latest + steps: + - name: Cache Pub hosted dependencies + uses: actions/cache@0c45773b623bea8c8e75f6c82b208c3cf94ea4f9 + with: + path: "~/.pub-cache/hosted" + key: "os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:web_generator;commands:analyze" + restore-keys: | + os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:web_generator + os:ubuntu-latest;pub-cache-hosted;sdk:dev + os:ubuntu-latest;pub-cache-hosted + os:ubuntu-latest + - name: Setup Dart SDK + uses: dart-lang/setup-dart@0a8a0fc875eb934c15d08629302413c671d3f672 + with: + sdk: dev + - id: checkout + name: Checkout repository + uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 + - id: web_generator_pub_upgrade + name: web_generator; dart pub upgrade + run: dart pub upgrade + if: "always() && steps.checkout.conclusion == 'success'" + working-directory: web_generator + - name: "web_generator; dart analyze --fatal-infos ." + run: dart analyze --fatal-infos . + if: "always() && steps.web_generator_pub_upgrade.conclusion == 'success'" + working-directory: web_generator + needs: + - job_001 + - job_002 + - job_003 + - job_004 + - job_005 + - job_006 + - job_007 + - job_008 + - job_009 + - job_010 + - job_011 + job_013: + name: "generate_all_and_analyze; Dart dev; PKG: web_generator; `dart bin/update_bindings.dart --generate-all`" + runs-on: ubuntu-latest + steps: + - name: Cache Pub hosted dependencies + uses: actions/cache@0c45773b623bea8c8e75f6c82b208c3cf94ea4f9 + with: + path: "~/.pub-cache/hosted" + key: "os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:web_generator;commands:command_2" + restore-keys: | + os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:web_generator + os:ubuntu-latest;pub-cache-hosted;sdk:dev + os:ubuntu-latest;pub-cache-hosted + os:ubuntu-latest + - name: Setup Dart SDK + uses: dart-lang/setup-dart@0a8a0fc875eb934c15d08629302413c671d3f672 + with: + sdk: dev + - id: checkout + name: Checkout repository + uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 + - id: web_generator_pub_upgrade + name: web_generator; dart pub upgrade + run: dart pub upgrade + if: "always() && steps.checkout.conclusion == 'success'" + working-directory: web_generator + - name: "web_generator; dart bin/update_bindings.dart --generate-all" + run: dart bin/update_bindings.dart --generate-all + if: "always() && steps.web_generator_pub_upgrade.conclusion == 'success'" + working-directory: web_generator + needs: + - job_001 + - job_002 + - job_003 + - job_004 + - job_005 + - job_006 + - job_007 + - job_008 + - job_009 + - job_010 + - job_011 + job_014: name: "dart_fixes; Dart main; PKG: web; `dart fix --compare-to-golden test_fixes`" runs-on: ubuntu-latest steps: @@ -461,3 +545,5 @@ jobs: - job_009 - job_010 - job_011 + - job_012 + - job_013 diff --git a/mono_repo.yaml b/mono_repo.yaml index 5b9a450d..3b7023b5 100644 --- a/mono_repo.yaml +++ b/mono_repo.yaml @@ -9,3 +9,4 @@ merge_stages: - unit_test - dart_fixes - generate_and_analyze + - generate_all_and_analyze diff --git a/tool/ci.sh b/tool/ci.sh index ef15b46c..667ea23f 100755 --- a/tool/ci.sh +++ b/tool/ci.sh @@ -75,6 +75,10 @@ for PKG in ${PKGS}; do echo 'dart bin/update_bindings.dart' dart bin/update_bindings.dart || EXIT_CODE=$? ;; + command_2) + echo 'dart bin/update_bindings.dart --generate-all' + dart bin/update_bindings.dart --generate-all || EXIT_CODE=$? + ;; format) echo 'dart format --output=none --set-exit-if-changed .' dart format --output=none --set-exit-if-changed . || EXIT_CODE=$? diff --git a/web_generator/CHANGELOG.md b/web_generator/CHANGELOG.md index 69a023a3..0e3110a8 100644 --- a/web_generator/CHANGELOG.md +++ b/web_generator/CHANGELOG.md @@ -3,9 +3,9 @@ - Initial separation of `web_generator` from `web`. - New IDL interface `RHL` added. `ExtendedAttribute` idl interface updated to expose its `rhs` property and `Interfacelike` idl interface updated to expose - `extAttrs` property. The generator now adds a + `extAttrs` property. The generator now adds a `JS(LegacyNamespace.$extensionTypeName)` annotation on `JS` objects if they've an IDL extended attribute `[LegacyNamespace=Foo]` defined in their IDL - description. - - \ No newline at end of file + description. +- Added `--generate-all` option to generate all bindings, including experimental + and non-standard APIs. diff --git a/web_generator/README.md b/web_generator/README.md index 0a90c095..d4afedb3 100644 --- a/web_generator/README.md +++ b/web_generator/README.md @@ -75,6 +75,17 @@ definitions: emit code that is standards track and is not experimental to reduce the number of breaking changes. +## Generate all bindings + +To ignore the compatibility data and emit all members, run: + +```shell +dart bin/update_bindings.dart --generate-all +``` + +This is useful if you want to avoid having to write bindings manually for some +experimental and non-standard APIs. + ## Web IDL versions Based on: diff --git a/web_generator/bin/update_bindings.dart b/web_generator/bin/update_bindings.dart index 0735b5b7..2b18dfb2 100644 --- a/web_generator/bin/update_bindings.dart +++ b/web_generator/bin/update_bindings.dart @@ -78,9 +78,14 @@ $_usage'''); }; // Run app with `node`. + final generateAll = argResult['generate-all'] as bool; await _runProc( 'node', - ['main.mjs', Platform.script.resolve('../../web/lib/src').path], + [ + 'main.mjs', + '--output-directory=${Platform.script.resolve('../../web/lib/src').path}', + if (generateAll) '--generate-all', + ], workingDirectory: _bindingsGeneratorPath, ); @@ -232,4 +237,8 @@ ${_parser.usage}'''; final _parser = ArgParser() ..addFlag('update', abbr: 'u', help: 'Update npm dependencies') ..addFlag('compile', defaultsTo: true) - ..addFlag('help', negatable: false); + ..addFlag('help', negatable: false) + ..addFlag('generate-all', + negatable: false, + help: 'Generate bindings for all IDL definitions, including experimental ' + 'and non-standard APIs.'); diff --git a/web_generator/lib/src/bcd.dart b/web_generator/lib/src/bcd.dart index f4b5b3ba..bba1144e 100644 --- a/web_generator/lib/src/bcd.dart +++ b/web_generator/lib/src/bcd.dart @@ -15,12 +15,18 @@ import 'filesystem_api.dart'; class BrowserCompatData { static final Map> _eventHandlers = {}; + /// Whether to generate all the bindings regardless of property status. + static bool generateAll = false; + /// Returns whether [name] is an event handler that is supported in any /// interface. static bool isEventHandlerSupported(String name) => + generateAll || _eventHandlers[name]?.any((bcd) => bcd.shouldGenerate) == true; - static BrowserCompatData read() { + static BrowserCompatData read({required bool generateAll}) { + BrowserCompatData.generateAll = generateAll; + final path = p.join('node_modules', '@mdn', 'browser-compat-data', 'data.json'); final content = (fs.readFileSync( @@ -84,7 +90,7 @@ class BrowserCompatData { BCDInterfaceStatus? retrieveInterfaceFor(String name) => interfaces[name]; bool shouldGenerateInterface(String name) => - retrieveInterfaceFor(name)?.shouldGenerate ?? false; + generateAll || (retrieveInterfaceFor(name)?.shouldGenerate ?? false); } class BCDInterfaceStatus extends BCDItem { @@ -120,7 +126,8 @@ class BCDInterfaceStatus extends BCDItem { return _properties[name]; } - bool get shouldGenerate => standardTrack && !experimental; + bool get shouldGenerate => + BrowserCompatData.generateAll || (standardTrack && !experimental); } class BCDPropertyStatus extends BCDItem { @@ -128,7 +135,8 @@ class BCDPropertyStatus extends BCDItem { BCDPropertyStatus(super.name, super.json, this.parent); - bool get shouldGenerate => standardTrack && !experimental; + bool get shouldGenerate => + BrowserCompatData.generateAll || (standardTrack && !experimental); } abstract class BCDItem { diff --git a/web_generator/lib/src/dart_main.dart b/web_generator/lib/src/dart_main.dart index 5d97c07c..2fec9fb4 100644 --- a/web_generator/lib/src/dart_main.dart +++ b/web_generator/lib/src/dart_main.dart @@ -4,6 +4,7 @@ import 'dart:js_interop'; +import 'package:args/args.dart'; import 'package:code_builder/code_builder.dart' as code; import 'package:dart_style/dart_style.dart'; @@ -18,21 +19,27 @@ import 'util.dart'; // probably involve parsing the TC39 spec. void main(List args) async { - await _generateAndWriteBindings(args[0]); + final ArgResults argResult; + argResult = _parser.parse(args); + await _generateAndWriteBindings( + outputDirectory: argResult['output-directory'] as String, + generateAll: argResult['generate-all'] as bool); } -Future _generateAndWriteBindings(String dir) async { +Future _generateAndWriteBindings( + {required String outputDirectory, required bool generateAll}) async { const librarySubDir = 'dom'; - ensureDirectoryExists('$dir/$librarySubDir'); + ensureDirectoryExists('$outputDirectory/$librarySubDir'); - final bindings = await generateBindings(packageRoot, librarySubDir); + final bindings = await generateBindings(packageRoot, librarySubDir, + generateAll: generateAll); for (var entry in bindings.entries) { final libraryPath = entry.key; final library = entry.value; final contents = _emitLibrary(library).toJS; - fs.writeFileSync('$dir/$libraryPath'.toJS, contents); + fs.writeFileSync('$outputDirectory/$libraryPath'.toJS, contents); } } @@ -47,3 +54,11 @@ String _emitLibrary(code.Library library) { return DartFormatter(languageVersion: DartFormatter.latestLanguageVersion) .format(source.toString()); } + +final _parser = ArgParser() + ..addOption('output-directory', + mandatory: true, help: 'Directory where bindings will be generated to.') + ..addFlag('generate-all', + negatable: false, + help: 'Generate bindings for all IDL definitions, including experimental ' + 'and non-standard APIs.'); diff --git a/web_generator/lib/src/generate_bindings.dart b/web_generator/lib/src/generate_bindings.dart index e00f2992..efeb593b 100644 --- a/web_generator/lib/src/generate_bindings.dart +++ b/web_generator/lib/src/generate_bindings.dart @@ -69,11 +69,13 @@ Future>> _generateElementTagMap() async { } Future generateBindings( - String packageRoot, String librarySubDir) async { + String packageRoot, String librarySubDir, + {required bool generateAll}) async { final cssStyleDeclarations = await _generateCSSStyleDeclarations(); final elementHTMLMap = await _generateElementTagMap(); final translator = Translator( - packageRoot, librarySubDir, cssStyleDeclarations, elementHTMLMap); + packageRoot, librarySubDir, cssStyleDeclarations, elementHTMLMap, + generateAll: generateAll); final array = objectEntries(await idl.parseAll().toDart); for (var i = 0; i < array.length; i++) { final entry = array[i] as JSArray; diff --git a/web_generator/lib/src/main.mjs b/web_generator/lib/src/main.mjs index b60821e7..697c6caa 100644 --- a/web_generator/lib/src/main.mjs +++ b/web_generator/lib/src/main.mjs @@ -21,8 +21,8 @@ globalThis.idl = idl; globalThis.location = { href: `file://${process.cwd()}/` } globalThis.dartMainRunner = async function (main, args) { - const dir = process.argv[2]; - await main(dir); + const dartArgs = process.argv.slice(2); + await main(dartArgs); } async function scriptMain() { diff --git a/web_generator/lib/src/translator.dart b/web_generator/lib/src/translator.dart index d0a2c310..737dff2e 100644 --- a/web_generator/lib/src/translator.dart +++ b/web_generator/lib/src/translator.dart @@ -522,7 +522,7 @@ class _PartialInterfacelike { // TODO(srujzs): Should we handle other special operations, // unnamed or otherwise? For now, don't emit the unnamed ones and // do nothing special for the named ones. - if (operationName.isEmpty) break; + if (operationName.isEmpty) continue; } final isStatic = operation.special == 'static'; if (shouldQueryMDN && @@ -591,6 +591,7 @@ class _PartialInterfacelike { /// Given a [memberName] and whether it [isStatic], return whether it is a /// member that should be emitted according to the compat data. bool _shouldGenerateMember(String memberName, {bool isStatic = false}) { + if (BrowserCompatData.generateAll) return true; // Compat data only exists for interfaces and namespaces. Mixins and // dictionaries should always generate their members. if (type != 'interface' && type != 'namespace') return true; @@ -665,10 +666,11 @@ class Translator { static Translator? instance; Translator(this.packageRoot, this._librarySubDir, this._cssStyleDeclarations, - this._elementTagMap) { + this._elementTagMap, + {required bool generateAll}) { instance = this; docProvider = DocProvider.create(); - browserCompatData = BrowserCompatData.read(); + browserCompatData = BrowserCompatData.read(generateAll: generateAll); } void _addOrUpdateInterfaceLike(idl.Interfacelike interfacelike) { @@ -702,31 +704,31 @@ class Translator { ...library.partialInterfaces ]) { final name = interfacelike.name; - bool shouldGenerate; switch (interfacelike.type) { case 'interface': - shouldGenerate = browserCompatData.shouldGenerateInterface(name); + if (browserCompatData.shouldGenerateInterface(name)) { + _addOrUpdateInterfaceLike(interfacelike); + _usedTypes.add(interfacelike); + } break; case 'namespace': // Browser compat data doesn't document namespaces that only contain // constants. // https://github.com/mdn/browser-compat-data/blob/main/docs/data-guidelines/api.md#namespaces - shouldGenerate = browserCompatData.shouldGenerateInterface(name) || + if (browserCompatData.shouldGenerateInterface(name) || interfacelike.members.toDart - .every((member) => member.type == 'const'); + .every((member) => member.type == 'const')) { + _addOrUpdateInterfaceLike(interfacelike); + _usedTypes.add(interfacelike); + } break; case 'dictionary': - shouldGenerate = false; + if (BrowserCompatData.generateAll) markTypeAsUsed(name); break; default: throw Exception( 'Unexpected interfacelike type ${interfacelike.type}'); } - - if (shouldGenerate) { - _addOrUpdateInterfaceLike(interfacelike); - _usedTypes.add(interfacelike); - } } for (final interfacelike in [ ...library.interfaceMixins, diff --git a/web_generator/mono_pkg.yaml b/web_generator/mono_pkg.yaml index 0570b3a2..b9af430c 100644 --- a/web_generator/mono_pkg.yaml +++ b/web_generator/mono_pkg.yaml @@ -11,3 +11,6 @@ stages: - generate_and_analyze: - command: dart bin/update_bindings.dart - analyze: --fatal-infos . +- generate_all_and_analyze: + - command: dart bin/update_bindings.dart --generate-all + - analyze: --fatal-infos . From d2bdba99836010dbc116b70299c9d68f6550ecfb Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Fri, 13 Sep 2024 10:18:46 -0700 Subject: [PATCH 2/2] Make generateAll non-static --- web_generator/lib/src/bcd.dart | 50 ++++++++++++++------------- web_generator/lib/src/translator.dart | 6 ++-- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/web_generator/lib/src/bcd.dart b/web_generator/lib/src/bcd.dart index bba1144e..3889e37b 100644 --- a/web_generator/lib/src/bcd.dart +++ b/web_generator/lib/src/bcd.dart @@ -15,18 +15,12 @@ import 'filesystem_api.dart'; class BrowserCompatData { static final Map> _eventHandlers = {}; - /// Whether to generate all the bindings regardless of property status. - static bool generateAll = false; - /// Returns whether [name] is an event handler that is supported in any /// interface. static bool isEventHandlerSupported(String name) => - generateAll || _eventHandlers[name]?.any((bcd) => bcd.shouldGenerate) == true; static BrowserCompatData read({required bool generateAll}) { - BrowserCompatData.generateAll = generateAll; - final path = p.join('node_modules', '@mdn', 'browser-compat-data', 'data.json'); final content = (fs.readFileSync( @@ -52,7 +46,7 @@ class BrowserCompatData { for (final symbolName in api.symbolNames) { final apiInfo = api[symbolName] as Map; - final interface = BCDInterfaceStatus(symbolName, apiInfo); + final interface = BCDInterfaceStatus(symbolName, apiInfo, generateAll); if (interface._sourceFile.startsWith(globalsFilePrefix)) { // MDN stores global members e.g. `isSecureContext` in the same location // as the interfaces. These are not interfaces, but rather properties @@ -73,19 +67,24 @@ class BrowserCompatData { globals.forEach((name, apiInfo) { for (final globalInterface in globalInterfaces) { - globalInterface.addProperty(name, apiInfo); + globalInterface.addProperty(name, apiInfo, generateAll); } }); - return BrowserCompatData(Map.fromIterable( - interfaces, - key: (i) => (i as BCDInterfaceStatus).name, - )); + return BrowserCompatData( + Map.fromIterable( + interfaces, + key: (i) => (i as BCDInterfaceStatus).name, + ), + generateAll); } final Map interfaces; - BrowserCompatData(this.interfaces); + /// Whether to generate all the bindings regardless of property status. + bool generateAll = false; + + BrowserCompatData(this.interfaces, this.generateAll); BCDInterfaceStatus? retrieveInterfaceFor(String name) => interfaces[name]; @@ -96,13 +95,18 @@ class BrowserCompatData { class BCDInterfaceStatus extends BCDItem { final Map _properties = {}; - BCDInterfaceStatus(super.name, super.json) { + late final bool shouldGenerate; + + BCDInterfaceStatus(super.name, super.json, bool generateAll) { for (final symbolName in json.symbolNames) { - addProperty(symbolName, json[symbolName] as Map); + addProperty( + symbolName, json[symbolName] as Map, generateAll); } + shouldGenerate = generateAll || (standardTrack && !experimental); } - void addProperty(String property, Map compat) { + void addProperty( + String property, Map compat, bool generateAll) { // Event compatibility data is stored as `_event`. In order // to have compatibility data for `onX` properties, we need to replace such // property names. See https://github.com/mdn/browser-compat-data/blob/main/docs/data-guidelines/api.md#dom-events-eventname_event @@ -111,12 +115,12 @@ class BCDInterfaceStatus extends BCDItem { const eventSuffix = '_event'; if (property.endsWith(eventSuffix)) { property = 'on${property.replaceAll(eventSuffix, '')}'; - status = BCDPropertyStatus(property, compat, this); + status = BCDPropertyStatus(property, compat, this, generateAll); BrowserCompatData._eventHandlers .putIfAbsent(property, () => {}) .add(status); } else { - status = BCDPropertyStatus(property, compat, this); + status = BCDPropertyStatus(property, compat, this, generateAll); } _properties[property] = status; } @@ -125,18 +129,16 @@ class BCDInterfaceStatus extends BCDItem { if (isStatic) name = '${name}_static'; return _properties[name]; } - - bool get shouldGenerate => - BrowserCompatData.generateAll || (standardTrack && !experimental); } class BCDPropertyStatus extends BCDItem { final BCDInterfaceStatus parent; - BCDPropertyStatus(super.name, super.json, this.parent); + late final bool shouldGenerate; - bool get shouldGenerate => - BrowserCompatData.generateAll || (standardTrack && !experimental); + BCDPropertyStatus(super.name, super.json, this.parent, bool generateAll) { + shouldGenerate = generateAll || (standardTrack && !experimental); + } } abstract class BCDItem { diff --git a/web_generator/lib/src/translator.dart b/web_generator/lib/src/translator.dart index 737dff2e..11a72c40 100644 --- a/web_generator/lib/src/translator.dart +++ b/web_generator/lib/src/translator.dart @@ -591,7 +591,7 @@ class _PartialInterfacelike { /// Given a [memberName] and whether it [isStatic], return whether it is a /// member that should be emitted according to the compat data. bool _shouldGenerateMember(String memberName, {bool isStatic = false}) { - if (BrowserCompatData.generateAll) return true; + if (Translator.instance!.browserCompatData.generateAll) return true; // Compat data only exists for interfaces and namespaces. Mixins and // dictionaries should always generate their members. if (type != 'interface' && type != 'namespace') return true; @@ -723,7 +723,9 @@ class Translator { } break; case 'dictionary': - if (BrowserCompatData.generateAll) markTypeAsUsed(name); + if (Translator.instance!.browserCompatData.generateAll) { + markTypeAsUsed(name); + } break; default: throw Exception(