DocumentLayout.getRectForPosition fails on Flutter Hot Reload

[craigdfoster]

I'm getting a null check error in super_text.dart line 73 every time I do a flutter Hot Reload. (Even for utterly minor code changes)

ProseTextLayout get textLayout => RenderSuperTextLayout.textLayoutFrom(_textLayoutKey)!;

I'm using SuperReader and have a document underlay that renders using a subclassed DocumentLayoutLayerStatelessWidget. The underlay calls DocumentLayout.getRectForPosition which then leads to the failure above.

Tracing it in debugger reveals that renderTextLayout.state._paragraph is null which forces textLayoutFrom to return null on line 190

  static ProseTextLayout? textLayoutFrom(GlobalKey key) {
    final renderTextLayout = key.currentContext?.findRenderObject() as RenderSuperTextLayout?;
    if (renderTextLayout == null || renderTextLayout.state._paragraph == null) {
      return null;
    }

    return RenderParagraphProseTextLayout(
      richText: renderTextLayout.state.widget.richText,
      renderParagraph: renderTextLayout.state._paragraph!,
    );
  }

I'm using the latest code from the stable branch

[matthew-carroll]

@craigdfoster can you please provide a runnable minimal reproduction so that we can paste it in and repro the bug?

[craigdfoster]

Hi @matthew-carroll,

Thanks for getting back! I've pulled together a project for you... macosui_starter.zip

Once it's up and running just run a hot reload and you should see the exception in the debug console. Try changing the text on line 30 of script_widget.dart, hot reload and you'll see that the text won't update without a full restart.

Note that the issue is avoided if I don't use a document underlay. You can see this by commenting out line 98 of script_widget.dart.

Cheers!

[matthew-carroll]

@craigdfoster the repro code should be small enough to paste in this issue ticket. It should be nothing but the necessary SuperEditor configuration that causes the problem. We don't usually download peoples' code, or other repositories.

[craigdfoster]

OK, I've paired it down further. Please use the macosui package

Note that the issue appears to go away without the call to MacosTheme.of(context) which occurs in code AFTER the exception is thrown. I wasn't able to reproduce using Material Theme.

import 'package:flutter/material.dart';
import 'package:macos_ui/macos_ui.dart';
import 'package:super_editor/super_editor.dart';

Future<void> _configureMacosWindowUtils() async {
  const config = MacosWindowUtilsConfig();
  await config.apply();
}

Future<void> main() async {
  await _configureMacosWindowUtils();
  runApp(const OSXApp());
}

class OSXApp extends StatelessWidget {
  const OSXApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MacosApp(
      title: 'macosui_starter',
      theme: MacosThemeData.light(),
      darkTheme: MacosThemeData.dark(),
      themeMode: ThemeMode.system,
      home: const MainView(),
    );
  }
}

class MainView extends StatelessWidget {
  const MainView({super.key});

  @override
  Widget build(BuildContext context) {
    return MacosWindow(
      child: MacosScaffold(
        children: [
          ContentArea(
            builder: (context, scrollController) {
              return const MyReaderWidget();
            },
          ),
        ],
      ),
    );
  }
}

class MyReaderWidget extends StatefulWidget {
  const MyReaderWidget({super.key});

  @override
  State<MyReaderWidget> createState() => MyReaderWidgetState();
}

class MyReaderWidgetState extends State<MyReaderWidget> {
  late final Document _document;

  @override
  void initState() {
    super.initState();

    _document = MutableDocument(
      nodes: [
        ParagraphNode(
          id: "1",
          text: AttributedText("Paragraph 1"),
        ),
        ParagraphNode(
          id: "2",
          text: AttributedText("Paragraph 2"),
        ),
      ],
    );
  }

  @override
  Widget build(BuildContext context) {
    return SuperReader(
      documentLayoutKey: GlobalKey(),
      document: _document,
      documentUnderlayBuilders: [MyLayerBuilder()],
    );
  }
}

class MyLayerBuilder extends SuperReaderDocumentLayerBuilder {
  @override
  ContentLayerWidget build(
      BuildContext context, SuperReaderContext documentContext) {
    return MyDocumentLayer(documentContext.document);
  }
}

class MyDocumentLayer extends DocumentLayoutLayerStatelessWidget {
  const MyDocumentLayer(this.document, {super.key});

  final Document document;

  @override
  Widget buildWithDocumentLayout(
      BuildContext context, DocumentLayout documentLayout) {
    return Stack(
      children: [
        for (var node in document.nodes) ...[
          Positioned(
            top: documentLayout
                // NOTE: the Null Check error is thrown from within getRectForPosition
                .getRectForPosition(DocumentPosition(
                    nodeId: node.id, nodePosition: node.beginningPosition))!
                .top,
            left: 0,
            child: Text(
              node.id,
              // BUT: The issue goes away without the theme call which occurs after
              style: MacosTheme.of(context).typography.body,
            ),
          ),
        ]
      ],
    );
  }
}

[matthew-carroll]

@craigdfoster is this a SuperEditor issue, or a Mac OS package issue? One of the reasons that we ask for a truly isolated reproduction is so that it's clear that this is our problem, and not a problem caused by other packages. Can you please try to reproduce your issue exclusively with SuperEditor without any external packages?

[craigdfoster]

Truthfully, I don't think I can go any further without your help.

This is a very weird bug. I have super_editor working in a complex app with no issues under normal testing except for this one very specific problem where it fails only in a Dev context, after Hot Reload. I've not seen anything else like it. The exact same code will execute without issue after a fresh start or restart. This btw makes it a very time consuming problem because the app will ALWAYS fail after Hot Reload which really impacts development productivity.

I've already used a great deal of trial and error removing other parts of the configuration superfluous to this issue and I have already tried recreating it with Material but to no avail. I'm not sure what else to try in my code. There's no obvious connection between the macOS package and super_editor that would guide further trials.

But what I do know is that the error is originating from the super_editor code base and that the MacosTheme.of(context) call is perfectly valid where it is and that it works perfectly in all other contexts.

I feel I can progress this no further without help from someone who has a good working knowledge of the super_editor code.

Would you mind at least tracing it and seeing what you find?

[matthew-carroll]

@angelosilvestre can you try the provided example code with the Mac OS ui package and see if you can root cause this? My initial guess is something related to content layers.

[angelosilvestre]

@matthew-carroll The issue seems that calling MacosTheme.of(context) causes the layer to be built before the document layout.

As an InheritedWidget is being used, it rebuilds the widget when it changes. Looking into the _InheritedMacosTheme from the macos_ui package we have:

@override
bool updateShouldNotify(_InheritedMacosTheme old) => theme.data != old.theme.data;

So, it rebuilds the dependent widgets when its MacosThemeData changes. As MacosThemeData doesn't implement value equality, two different instances with the same values aren't equal. It seems that during the hot reload a new MacosThemeData is created.

As a test, I changed updateShouldNotify to always return false and the issue stopped.

It seems it isn't safe to use InheritedWidgets inside a DocumentLayoutLayerStatelessWidget, because it might cause widgets to build out of the order that ContentLayers expect. However, I was able to run the sample without issues using a DocumentLayoutLayerStatefulWidget:

class MyStatefulDocumentLayer extends DocumentLayoutLayerStatefulWidget {
  const MyStatefulDocumentLayer(this.document, {super.key});

  final Document document;

  @override
  DocumentLayoutLayerState<ContentLayerStatefulWidget, dynamic> createState() {
    return MyStatefulDocumentLayerState();
  }
}

class MyStatefulDocumentLayerState extends DocumentLayoutLayerState<MyStatefulDocumentLayer, List<LayoutInfo>> {
  @override
  List<LayoutInfo>? computeLayoutDataWithDocumentLayout(BuildContext context, DocumentLayout documentLayout) {
    final layoutData = <LayoutInfo>[
      for (var node in widget.document.nodes) ...[
        LayoutInfo(
          nodeId: node.id,
          top: documentLayout
              .getRectForPosition(DocumentPosition(nodeId: node.id, nodePosition: node.beginningPosition))!
              .top,
        )
      ]
    ];

    return layoutData;
  }

  @override
  Widget doBuild(BuildContext context, List<LayoutInfo>? layoutData) {
    if (layoutData == null) {
      return const SizedBox();
    }
    return Stack(
      children: [
        for (var item in layoutData) ...[
          Positioned(
            top: item.top,
            left: 0,
            child: Text(
              item.nodeId,
              style: MacosTheme.of(context).typography.body,
            ),
          ),
        ]
      ],
    );
  }
}

class LayoutInfo {
  LayoutInfo({
    required this.nodeId,
    required this.top,
  });

  final String nodeId;
  final double top;
}

@craigdfoster Could you please try this sample code and see if it works for you?

[matthew-carroll]

The issue seems that calling MacosTheme.of(context) causes the layer to be built before the document layout.

If this is the case, why does a stateful layer work but a stateless layer doesn't?

Also, if this is happening during hot reload, the document layout should already be built and cached. So why are we having a problem building a layer, even if the document layout rebuild out of order?

[angelosilvestre]

@matthew-carroll I looked into the stateful layer and we have this:

if (contentLayers != null && !contentLayers.renderObject.contentNeedsLayout) {
  _layoutData = computeLayoutData(contentElement, contentLayout);
}

So the method that access the document layout isn't called when we the layout is dirty. We could do something similar in the stateless build method, returning a SizedBox if the layout is dirty, or passing a nullable DocumentLayout to buildWithDocumentLayout, so apps can decide what to do if the layout isn't available.

What do you think?

[matthew-carroll]

So the method that access the document layout isn't called when we the layout is dirty

I don't know what this means. What is "the method that access the document layout" and where does your snippet show something related to "when the layout is dirty"?

[craigdfoster]

@craigdfoster Could you please try this sample code and see if it works for you?

Yes @angelosilvestre, that all works as you've described on my end.

FYI, capitalising on your observation about MacosThemeData and value equality, I've found the most simple and lightweight workaround for the time being is to prevent new MacosThemeData objects being created on the widget tree rebuild as below...

class OSXApp extends StatelessWidget {
  OSXApp({super.key});

  final lightTheme = MacosThemeData.light();
  final darkTheme = MacosThemeData.dark();

  @override
  Widget build(BuildContext context) {
    return MacosApp(
      title: 'macosui_starter',
      theme: lightTheme,
      darkTheme: darkTheme,
      themeMode: ThemeMode.system,
      home: const MainView(),
    );
  }
}

MacosThemeData should implement value equality. I'll raise an issue with the macos_ui team.

Thank you both very much for your help! Merry Christmas :)

[angelosilvestre]

I don't know what this means. What is "the method that access the document layout" and where does your snippet show something related to "when the layout is dirty"?

@matthew-carroll

In the stateless version, buildWithDocumentLayout accesses the document layout. In the stateful version, only computeLayoutDataWithDocumentLayout accesses the document layout, and this method isn't called when the content needs layout.

In the original sample code we have:

Positioned(
  top: documentLayout
      // NOTE: the Null Check error is thrown from within getRectForPosition
      .getRectForPosition(DocumentPosition(
          nodeId: node.id, nodePosition: node.beginningPosition))!
      .top,
  left: 0,
  child: Text(
    node.id,
    // BUT: The issue goes away without the theme call which occurs after
    style: MacosTheme.of(context).typography.body,
  ),
),

getRectForPosition ends up calling RenderSuperTextLayout.textLayoutFrom with a null assertion (! operator) , which has this guard clause:

final renderTextLayout = key.currentContext?.findRenderObject() as RenderSuperTextLayout?;
if (renderTextLayout == null || renderTextLayout.state._paragraph == null) {
  return null;
}

RenderSuperTextLayout nullifies its _paragraph when markNeedsLayout is called. During hot reload, it seems markNeedsLayout is being called, and the layer is being built while we have a null _paragraph.

[matthew-carroll]

I'm not sure why the issue happens for stateless and not stateful, but I think it's likely that the root problem is in the ContentLayersElement.

In the ContentLayersElement, try overriding the reassemble() method and in that method call _temporarilyForgetLayers() and see if that solves the issue.

[angelosilvestre]

In the ContentLayersElement, try overriding the reassemble() method and in that method call _temporarilyForgetLayers() and see if that solves the issue.

@matthew-carroll I tried that but we get the same error.

[matthew-carroll]

Ok. I'll need to take a look at this.