Migrated from flutter/flutter#81748
This issue was originally filed against the official flutter_markdown package.
The package has since been discontinued and ownership transferred to flutter_markdown_plus.
The manner in which the TaskListSyntax inline parser object is added to the list of inline parsers for the document when parsing Markdown tags can have undesirable side-effects. Each time the MarkdownWidget's dependencies change or the widget is updated, the method _parseMarkdown is called. _parseMarkdown creates an instance of Document from the markdown package as follows (code from widget.dart in flutter_markdown package):
final md.Document document = md.Document(
extensionSet: widget.extensionSet ?? md.ExtensionSet.gitHubFlavored,
inlineSyntaxes: (widget.extensionSet?.inlineSyntaxes ?? [])
..add(TaskListSyntax())
..map((syntax) => syntax),
encodeHtml: false,
);
These are the problems I see with this implementation:
- Minor Issue: When an ExtensionSet object is passed to the MarkdownWidget class, the _parseMarkdown method is modifying the list of inlineSyntaxes of the supplied extension set. The caller of the Markdown and MarkdownBody widgets is probably not aware that the extension set provided has been modified. If the ExtensionSet object is created before calling Markdown or MarkdownBody and not used again the side-effect is minimal.
- Serious Issue: When using one of the defined extension sets from the markdown package, ExtensionSet.none, ExtensionSet.commonMark, ExtensionSet.gitHubWeb, or ExtensionSet.gitHubFlavored, since these are static properties, their respective inlineSyntax lists are being modified with a new instance of TaskListSyntax being added to their list of inlineSyntaxes every time any MarkdownWidget instance has a dependency change or is updated and calls _parseMarkdown. Since the default extension set is ExtensionSet.gitHubFlavored this behavior will affect a significant number of users of the flutter_markdown package.
- Minor Issue: By modifying the inlineSyntax list to the input extension set in this manner and passing it as the inlineSyntaxes parameter to the Document class has the effect of specifying the exact same list of inline syntax parsers to Document object being constructed twice, once in the inlineSyntaxes list and a second time in ExtensionSet.inlineSyntaxes. Fortunately, the Document constructor creates a set from these two lists so there aren't duplicate inline parsers in the Document's inlineSyntaxes. While this isn't a serious issue it does make the code less intuitive and understandable. (Document constructor code from document.dart in markdown v2.1.7 package.)
Document({
Iterable<BlockSyntax> blockSyntaxes,
Iterable<InlineSyntax> inlineSyntaxes,
ExtensionSet extensionSet,
this.linkResolver,
this.imageLinkResolver,
this.encodeHtml = true,
}) : extensionSet = extensionSet ?? ExtensionSet.commonMark {
_blockSyntaxes
..addAll(blockSyntaxes ?? [])
..addAll(this.extensionSet.blockSyntaxes);
_inlineSyntaxes
..addAll(inlineSyntaxes ?? [])
..addAll(this.extensionSet.inlineSyntaxes);
}
- Minor Issue: The line of code
..map((syntax) => syntax), in building the list of inlineSyntaxes to be passed to the Document seems to be superfluous. This is a nit as this method of defining the list of inlineSyntaxes is probably not going to remain in a fix for the problems identified.
Possible Solution In reading through the markdown package documentation, it seems like the correct method for including additional inlineSyntax object to be included in the parsing of a Markdown document is to specify them in the Document's inlineSyntaxes parameter and don't modify the extension set's list of inlineSyntaxes. A possible solution to the problem could be this:
final md.Document document = md.Document(
extensionSet: widget.extensionSet ?? md.ExtensionSet.gitHubFlavored,
inlineSyntaxes: [TaskListSyntax()],
encodeHtml: false,
);
While this fixes the immediate problem identified, it prohibits allowing calls to include additional inline syntax handlers. But that is a different issue.
The manner in which the TaskListSyntax inline parser object is added to the list of inline parsers for the document when parsing Markdown tags can have undesirable side-effects. Each time the MarkdownWidget's dependencies change or the widget is updated, the method _parseMarkdown is called. _parseMarkdown creates an instance of Document from the markdown package as follows (code from widget.dart in flutter_markdown package):
These are the problems I see with this implementation:
..map((syntax) => syntax),in building the list of inlineSyntaxes to be passed to the Document seems to be superfluous. This is a nit as this method of defining the list of inlineSyntaxes is probably not going to remain in a fix for the problems identified.Possible Solution In reading through the markdown package documentation, it seems like the correct method for including additional inlineSyntax object to be included in the parsing of a Markdown document is to specify them in the Document's inlineSyntaxes parameter and don't modify the extension set's list of inlineSyntaxes. A possible solution to the problem could be this:
While this fixes the immediate problem identified, it prohibits allowing calls to include additional inline syntax handlers. But that is a different issue.