no need to register in buildCallback if you already have it in layoutCallback

There is no need to store the observer, also unlistener doesn't need to be an array. I recommend

this.resizeObserverUnlistener_ = null;

Then in layoutCallback and unlayoutCallback you check the resizeObserverUnlistener_

Use this.win instead.

It's unclear whether the updateFontSize_ is invoked by the resizeObserver or layoutCallback here. Better do

layoutCallback
// check called
change size
// check called

I added the new tests, is this test more clear?

expect(setupUpdateFontSizeSpy).to.be.called; once checks if the function has been called before. Use
calledOnce and calledTwice to be more specific

These <!-- ## --> blocks are customarily elaborated for amp.dev code samples, but this file will not be ported to amp.dev. I'd recommend either adding this example in that repo or removing annotations from this file.

nit: new line eof

please flip to save a level of indentation:

if (!this.win.ResizeObserver) {
  return;
}

Would it make more sense to just pull this part into the same if check as the following one? like

You still want to early return to save a level of indentation.

if (!this.win.ResizeObserver || this.resizeObserverUnlistener_) {
  return;
}

In this case we want to call the final return statement whether or not this if block is true, this specifically relates to deciding whether we should attach a new observer, would it be better to extract the logic which attaches the listener to a helper method? so that the layout callback would look like this?

if (this.win.ResizeObserver && this.resizeObserverUnlistener_ === null) {
    attachNewResizeObserver();
}
return this.mutateElement(() => {
   this.updateFontSize_();
});

Oh I see, apologies for missing that. In that case either way works :)

Sounds good, Thanks!

Should this be initialized to () => {} so that we can never accidentally call null in the unlayoutCallback?

it is also used as a signal to tell if resizeObserver has been initialized. I recommend initialize it to null but add the check to unlayoutCallback

Works for me.

I set this to null to indicate that a resizeObserver has not been attached yet.

Ah! I replied before I saw your comment, SGTM.

If this is moving to an async method, you can use await syntax instead of relying on the thenable directly.

nit: `{?unlistenDef}

also set this.resizeObserverUnlistener_ back to null.

Are these HTML comments necessary in the doc?

I added them just for clarity and readability for anyone who would be looking at this file, but I can pull them out if they aren't necessary.

Should this be function? Where is unlistenDef defined?

UnlistenDef is defined in amp.extern.js (https://github.com/ampproject/amphtml/blob/master/build-system/externs/amp.extern.js#L414-L418) and refers to a function which specifically removes a listener, in this case the resizeObserver. I think its just a shade more precise than the function type declaration for this usage.

Oh I see - then should this be capitalized?

FMI: Why is this line added to test that the component renders?

I initially added this because without this call the ResizeObserver unlistener, was not being correctly triggered, however after debugging this appears to have been related to a different issue so I have removed this line in this test.

Why 90? Could be helpful to add an inline comment on why this value is meaningful.

In this case 90 was arbitrarily chosen, i'll make it 100 and then add a comment to indicate why it was chosen.

I wasn't sure if it was intentionally related to the throttle rate. Thanks for adding a comment :)

The idea is to wait for the ResizeObserver to fire, but not until the next function call due to throttle. So any value less than 100 should be good.

It turns out 100ms exactly is a border case which can pass but its not clear whats happening so I am gonna re-set it to 90 and adjust my comments accordingly.

Oh got it, thanks both for clarifying!