Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[web] Add views proxy and getInitialData. #49320

Merged
merged 8 commits into from
Jan 26, 2024

Conversation

ditman
Copy link
Member

@ditman ditman commented Dec 21, 2023

Adds a Dart API so Application/Plugin programmers can retrieve the initialData configuration value that may be passed when adding a view from JS in a multiViewEnabled app.

When adding a view to an app like this:

flutterApp.addView({
  hostElement: someElement,
  initialData: {
    randomUUID: globalThis.crypto.randomUUID(),
  }
});

initialData can be accessed from Dart by defining a JS-interop class like:

import 'dart:js_interop';

// The JS-interop definition of the `initialData` object passed to the views of this app.
@JS()
@staticInterop
class InitialData {}

/// The attributes of the [InitialData] object.
extension InitialDataExtension on InitialData {
  external String? get randomUUID;
}

And then, from the code of the application:

...
  Widget build(BuildContext context) {
    final int viewId = View.of(context).viewId;
    final InitialData? data = ui_web.views.getInitialData(viewId) as InitialData?;
    return Text('${data?.randomUUID}');
  }
...

Testing

Will add unit tests once naming is sorted out :)

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added the platform-web Code specifically for the web engine label Dec 21, 2023
@ditman ditman force-pushed the web-only-initial-data-getter branch from b400ae2 to bc1bdce Compare December 22, 2023 03:12
@ditman ditman changed the title [web] Add viewsProxy and getInitialData. [web] Add views proxy and getInitialData. Dec 22, 2023
@ditman ditman changed the title [web] Add views proxy and getInitialData. [web] Add views proxy and getInitialData. Dec 22, 2023
@wanjm
Copy link

wanjm commented Dec 22, 2023

beside web, other platform should also need to get the initData, can we add a property to FlutterView?

@ditman
Copy link
Member Author

ditman commented Jan 2, 2024

beside web, other platform should also need to get the initData, can we add a property to FlutterView?

@wanjm please, create a new issue with a feature request so @goderbauer knows you want this too for other platforms (what other platforms?).

If the framework ever supports initialData or similar, we'll harmonize the web implementation to match the framework API. For now this is only supported on the web, and in multi-view mode, and so the API lives in web-only specific bits.

@chandanmallick19

This comment was marked as off-topic.

Comment on lines 26 to 27
Object? getInitialData(int viewId) {
return _viewManager.getOptions(viewId)?.initialData;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before we add official APIs for initial data, I still would like us to explore without it. Here's an example of how I could see this done:

Let's have a getViewOptions API:

// Of course, we would need to make `JsFlutterViewOptions` public thru `dart:ui_web`.
JsFlutterViewOptions? getViewOptions(int viewId) {
  return _viewManager.getOptions(viewId);
}

And then users can define their custom JsFlutterViewOptions implementation:

import 'dart:js_interop';
import 'dart:ui_web' as ui_web;

// The JS-interop definition of the view options object passed when creating a flutter view.
@JS()
@staticInterop
// I'm not sure if `extends` or `implements` is better here.
class MyJsFlutterViewOptions implements ui_web.JsFlutterViewOptions {}

extension MyJsFlutterViewOptionsExtension on MyJsFlutterViewOptions {
  // Users can put properties directly here
  external String? get randomUUID;

  // or package everything into an `initialData` property
  external MyJsInitialData get initialData;
}

This way, users also get access to the view's hostElement, sizeConstraints, etc.

@ditman
Copy link
Member Author

ditman commented Jan 4, 2024

@ditman ditman force-pushed the web-only-initial-data-getter branch from 1769a6a to 624c481 Compare January 10, 2024 19:08
@ditman

This comment was marked as outdated.

@mdebbar
Copy link
Contributor

mdebbar commented Jan 11, 2024

After giving it some more thought, I'm not opposed to having an API for initialData. It makes the feature easily discoverable by new users and makes documentation straightforward.

@ditman ditman force-pushed the web-only-initial-data-getter branch from 23c5fcf to 02046d3 Compare January 13, 2024 00:12
@ditman
Copy link
Member Author

ditman commented Jan 16, 2024

Adding some unit tests and prepping to land this.

@ditman ditman force-pushed the web-only-initial-data-getter branch from 02046d3 to 436e813 Compare January 16, 2024 23:17
@ditman ditman marked this pull request as ready for review January 17, 2024 02:18
@ditman
Copy link
Member Author

ditman commented Jan 17, 2024

I think this is ready to go, PTAL @mdebbar!

@ditman ditman requested a review from mdebbar January 17, 2024 02:19
Copy link
Contributor

@mdebbar mdebbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

/// can add their own custom HTML elements (for example: file inputs for the
/// file_selector plugin).
JSAny? getHostElement(int viewId) {
return _viewManager.getOptions(viewId)?.hostElement as JSAny?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is as JSAny? necessary? I would expect this to work:

Suggested change
return _viewManager.getOptions(viewId)?.hostElement as JSAny?;
return _viewManager.getOptions(viewId)?.hostElement;

Copy link
Member Author

@ditman ditman Jan 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

analyzer is not happy without the cast:

A value of type 'DomElement?' can't be returned from the method
'getHostElement' because it has a return type of 'JSAny?'.
(return_of_invalid_type)

(It's the same if I make the return a JSObject?)

@ditman ditman added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 26, 2024
@auto-submit auto-submit bot merged commit 1eea728 into flutter:main Jan 26, 2024
27 checks passed
@ditman ditman deleted the web-only-initial-data-getter branch January 26, 2024 02:37
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 26, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jan 26, 2024
…142290)

flutter/engine@9142fc4...8a81e53

2024-01-26 skia-flutter-autoroll@skia.org Roll Dart SDK from 00784f1f22b5 to 6f8bcd5b48ab (1 revision) (flutter/engine#50074)
2024-01-26 ditman@gmail.com [web] Add `views` proxy and `getInitialData`. (flutter/engine#49320)
2024-01-26 ditman@gmail.com [web] Prevent re-rendering disposed views when the engine hot restarts. (flutter/engine#49958)
2024-01-26 skia-flutter-autoroll@skia.org Roll Dart SDK from 2fb950853f06 to 00784f1f22b5 (3 revisions) (flutter/engine#50068)
2024-01-26 matanlurey@users.noreply.github.com Avoid sizing `ImageReaderSurfaceProducer` smaller than 1x1 (flutter/engine#50066)
2024-01-26 jonahwilliams@google.com Use clamp sampling mode in external texture. (flutter/engine#50063)
2024-01-26 john@johnmccutchan.com Reland Optimizations for TLHC frame rate and jank (flutter/engine#50065)
2024-01-25 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from Igt2rE-R6rgfmTRaF... to WHlwlOwznFknNm5IS... (flutter/engine#50059)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from Igt2rE-R6rgf to WHlwlOwznFkn

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC jacksongardner@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@albertolina
Copy link

Hello! Is this feature already live? I can't seem to find any docs on it. Thanks

@ditman
Copy link
Member Author

ditman commented Feb 7, 2024

I can't seem to find any docs on it.

@albertolina It's been pushed to flutter master so it can be validated by some internal customers, but it hasn't been announced or documented yet, so: patience please!

@albertolina
Copy link

Thank you for clarifying @ditman! Much appreciated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants