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

Draft of how widgets could be organized #8035

Merged
merged 4 commits into from
Sep 12, 2024
Merged

Conversation

jonasfj
Copy link
Member

@jonasfj jonasfj commented Sep 6, 2024

This also makes widget loading a deferred, or at-least offers the option.

It's possible we should do a pkg/web_widget/ package and put all widgets in there.


Then maybe, move app/lib/frontend/dom/dom.dart into a pkg/html_as_code/ package.

Or we could choose to only organize into packages in the cases where we want to publish.

Do we really have much motivation to use different versions of libraries on the server and in the client. I suspect this was mostly relevant when pana and dartdoc were fully embedded in the server.

We currently only use dartdoc as a subprocess, and there could be a future where pana isn't embedded any. We'd need to formalize the data structures a bit, but keeping pana is probably not so bad 🤣

@jonasfj
Copy link
Member Author

jonasfj commented Sep 6, 2024

I don't know how many widgets we'd like to defer loading on, but something like completion that has to be activated and which also has to fetch completion data might as well be deferred.

I'd assume the deferred JS is quickly loaded on subsequent page loads anyways :D

Copy link
Collaborator

@isoos isoos left a comment

Choose a reason for hiding this comment

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

Small nits.

),
];

Future<void> setupWidgets() async {
Copy link
Collaborator

Choose a reason for hiding this comment

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

setupWidgets became asynchronous, but in script.dart we still call it as a sync method, no indication that the future is not awaited.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm making it void because I don't want people to await it :D

setupWidgets does what it does, you don't need to know if it failed, it'll report errors in console.

pkg/web_app/lib/src/widget/widget.dart Outdated Show resolved Hide resolved
@isoos
Copy link
Collaborator

isoos commented Sep 10, 2024

It's possible we should do a pkg/web_widget/ package and put all widgets in there.
[...] Or we could choose to only organize into packages in the cases where we want to publish.

Let's do separate packages only when we want to publish them (or have other compelling reasons for them).

I'd assume the deferred JS is quickly loaded on subsequent page loads anyways :D

I think that should be the case, at least I haven't seen anything else to the contrary.

@isoos
Copy link
Collaborator

isoos commented Sep 10, 2024

Also: needs to update the size check (which is nice as we know that the deferred loading works!).

@jonasfj jonasfj merged commit f24755c into dart-lang:master Sep 12, 2024
32 checks passed
@jonasfj jonasfj deleted the widget-draft branch September 12, 2024 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants