Modularization #645

tneotia · 2021-04-29T13:15:43Z

tneotia
Apr 29, 2021

8000

This is a discussion on how to tackle the issue of modularization. Some have said to split the package into two with a "_core" version and the full-fat "flutter_html". I had a different sort of idea which I will outline below.

Initial thought process

I was originally thinking about the differences between customRender and customImageRender, and if the latter is really necessary now that customRender will soon have a similar functionality. I think it's great that we provide customization for images, but that bears the question - why don't we do the same for all the ReplacedElements? We should offer customization for audio, video, etc to allow from asset or memory.

That was my starting point. Then I began to think how we could accomplish this without adding a ton of extra parameters like customAudioRender, customVideoRender, and then I realized that the solution I came up with could also serve as a great way to modularize this library. My proposal is as follows:

Proposal

Remove customImageRender entirely. That's it, really. Users have the RenderContext exposed to them when making a customRender implementation, so they can do the same thing. Sure, it will require a little bit more code (doing context.tree.element?.attributes instead of just attributes), but it removes a redundancy (at least I think it's a redundancy to have customImageRender).

How can this modularize the package?

We remove all the ReplacedElements and LayoutElements that use external dependencies (and remove the dependencies too). Then we can create a few different unlisted packages like flutter_html_audio and flutter_html_table. Each of these packages has their own pubspec with dependencies on flutter_html and whichever dependencies are needed to render their specific tag. Each package exposes one CustomRender to the user for that specific tag.
We can include a couple CustomRenderMatchers for easy plug-and-play functionality, like videoMatcher() or audioMatcher()

If the user doesn't include these renders, the core package returns an EmptyContentElement which eventually will be removed from the tree. If the user does include these renders, then the core package will have all the details to construct the widget.

The user can also choose to bypass the unlisted packages and just create their own render with their own dependencies.

An example API is shown below.

Example

import 'package:flutter_html_video/flutter_html_video.dart';

Html(
   data: htmlData
   customRender: {
      videoMatcher(): buildVideoElement(), //from flutter_html_video package
      svgMatcher(): CustomRender.fromWidget(widget: (context, buildChildren) {
         //return some widget here
      }),
   }
);

There might be holes in this proposal, or you may have a better one. I just wanted to get my idea down into a clear and concise format. Let me know your thoughts!

dreamer2q · 2021-09-13T09:58:47Z

dreamer2q
Sep 13, 2021

Your ideas are great.

But some questions might confuse me

Since you have split flutter_html into many packages, monorepo or multi-repo? How do you plan to maintain them?
You have used customRender for ReplacedElements, why not something like Html.register(videoMatcher(),buildVideoElement()) by having a static customRenderMap variables instead of filling customRender each time the User use Html.

2 replies

erickok Sep 13, 2021

The plan is to maintain them in the same repository, just as separate packages, They will be maintained by us and will offer (only) a limited set of extensions - probably only the video, audio and iframe ones we already support right now. I don't think we should offer variants, such as differnet video libraries, from this monorepo.

As for the suggestion of a central 'registry', I think it is interesting. Indeed it'd allow the use of the Html widget without the need for the 'boilerplate'. On the other hand, it contradicts the declarative approach a bit of how Flutter widgets work and we'd now have 2 APIs (custom render and the central registry) to do the same thing, which I don't like so much.

tneotia Sep 14, 2021
Author

I'd say for the second one users can implement a global widget they can use throughout their project, if they need to use the Html widget multiple times but keep the same parameters. Constraining the customRender to be static and universal for every single widget doesn't sound like a great idea, even in my own app the widget has different uses and I use different customRender to accomplish them.

Carlit0 · 2021-10-12T16:13:51Z

Carlit0
Oct 12, 2021

Hi, I am very glad that the modularization issue is already mentioned here! Thanks @tneotia!

Your approach @tneotia is interesting, however there will be a lot of breaking changes plus duplicate package maintenance!

I thought about this issue and came up with a different approach:
Why don't we use a little bit of boilerplate code to implement different Html widgets like HtmlCore (or whatever) and then use the power of import ... show HtmlCore to only show or hide those widgets?
Of course, this would have to be well documented. But this also applies to your approach :)
What do you think?

6 replies

Carlit0 Oct 12, 2021

Yes, Dart conditional import isn't useful at all.
I mean the show / hide keywords shown here: https://dart.dev/guides/language/language-tour#using-libraries

We now generate different classes (for all the use cases), that all implement or extend from each other.
Then the user of the package can decide which classes the want to show.
Basically it is very similar to your approach without the need for all the extra packages, and less to none breaking changes.

We could come up with a HtmlCore class from which all other classes extend :)

To me, this modularization feature is still a feature and therefore should be optional without having to think about it :)

tneotia Oct 12, 2021
Author

Ahhh I see, so when the user hides those classes they are not compiled and therefore removed from the final application. This sounds like a great idea, I had a PR created earlier for separate packages but I like this one much better. If I have time I will try and update my PR with this implementation!

erickok Oct 12, 2021

Except, to the best of my knowledge, the moment you'd include a package such as, say, chewie, you still need to deal with the dependency (and potential dependency conflicts) even if you don't import it's classes, because it's in your pubspec. Even more, native (Android or iOS) dependencies would get pulled in regardless, simply because you reference them.

Carlit0 Oct 12, 2021

@erickok
Although I was aware of the dependency situation, I didn't know that Plugin packages get pulled just by referencing them! What a bummer!

tneotia Oct 12, 2021
Author

@erickok in my understanding Dart is smart about this because using things like conditional imports you can build apps for web even though there are dependencies that would otherwise prevent building for web. However I don't know if the same concept applies for the hide or show, probably not because you'd still be importing the file that imports those dependencies....

Uh oh!

Modularization #645

Uh oh!

Uh oh!

tneotia Apr 29, 2021

Initial thought process

Proposal

Example

Replies: 2 comments · 8 replies

Uh oh!

Uh oh!

dreamer2q Sep 13, 2021

Uh oh!

erickok Sep 13, 2021

Uh oh!

tneotia Sep 14, 2021 Author

Uh oh!

Carlit0 Oct 12, 2021

Uh oh!

Carlit0 Oct 12, 2021

Uh oh!

tneotia Oct 12, 2021 Author

Uh oh!

erickok Oct 12, 2021

Uh oh!

Carlit0 Oct 12, 2021

Uh oh!

tneotia Oct 12, 2021 Author

tneotia
Apr 29, 2021

Replies: 2 comments 8 replies

dreamer2q
Sep 13, 2021

tneotia Sep 14, 2021
Author

Carlit0
Oct 12, 2021

tneotia Oct 12, 2021
Author

tneotia Oct 12, 2021
Author