delegate.updateModifier is not a function

[lolmaus]

Hey @chadian!

Seeing this when using the ember-fill-up modifier.

Is there a reason to use a custom modifier manager?

[lolmaus]

The crash happens on this line:
https://github.com/emberjs/ember.js/blob/v3.20.0-beta.1/packages/@ember/-internals/glimmer/lib/modifiers/custom.ts#L172

I've linked to latest tagged version of the file which happens to be v3.20.0-beta.1 at the moment, but I'm seeing the error in Ember 3.13.2.

[lolmaus]

Here's my modifier implementation based on the modern ember-modifier:

import Modifier from 'ember-modifier';
import { action } from '@ember/object';

export default class DidResizeModifier extends Modifier {
  detector = null;

  constructor(owner, args) {
    super(owner,args);

    this.detector = owner.lookup('resize-detector:resize-observer');
  }

  @action
  onResize(element) {
    const onResize = this.args.positional[0];

    if (onResize) {
      return onResize(element);
    }
  }


  didInstall() {
    this.detector.listenTo(this.element, this.onResize);
  }

  didReceiveArguments() {
    this.onResize(this.element);
  }

  willRemove() {
    this.detector.removeListener(this.element, this.onResize);
  }
}

It accepts a positional arg, e. g. <img {{did-resize this.updateSrc}}>.

[chadian]

Hey @lolmau, I'll take a look this week. I want to come back and fix a few things in this addon. Also, in the past week it looks like there's another container query solution for ember: https://github.com/ijlee2/ember-container-query. I will still definitely look into fixing this though, thanks again for the report!

[lolmaus]

looks like there's another container query solution for ember: https://github.com/ijlee2/ember-container-query

Huh? I wonder if the author was aware of ember-fill-in when they were making another addon.

Looks like it relies on ember-did-resize-modifier which relies on element-resize-detector which does not use ResizeObserver.

I wonder if depending on element-resize-detector is justified over ResizeObserver.

[lolmaus]

CC @ijlee2.

[ijlee2]

@chadian @lolmaus Hello! Thanks for including me in this conversation.

Yep, I was aware of ember-fill-up when I wrote ember-container-query. I used Chad's addon at work (everyone on my team liked it) and, from this experience, began to understand how I might rewrite its core features using modifiers.

I used ember-did-resize-modifier because I've used the addon for some time and it's well tested and documented. Recently I found that there are a couple of addons that also look at how to write a modifier to observe resize. I hope to follow their development from time to time, to see if ember-did-resize-modifier continues to be a good choice.

[lolmaus]

Hey @ijlee2, thanks for chiming in!

What exactly were the reasons for rewriting?

[ijlee2]

Hmm, I'm not sure if this GitHub issue is a good place to share my reasons as they're unrelated to updateModifier. But I do want to engage in your conversation to show that my working on a new addon for container queries wasn't to offend Chad. I think we share a goal of wanting more people to use container queries in their app or addon. 🙂

A few months back, my team and I noticed that Percy snapshots would show ember-fill-up using the desktop breakpoint instead of the mobile one. For a while, we ignored this issue because our CSS wasn't great to begin with. When we switched to using CSS Grid, we still saw incorrect Percy snapshots.

Initially, I tried reading ember-fill-up's source code, but didn't get far in understanding how all pieces worked together. I began to work on using modifiers to recreate ember-fill-up, to see if it was Chad's addon or something else that was causing the problem. (The culprit turned out to be ember-qunit.)

PS. Chad, if you have concerns or questions, please feel free to reach out to me on Discord (ijlee2). I'd be happy to talk with you!

[chadian]

Hey @ijlee2!

Definitely appreciate the shout out. It's really cool to see the idea continue to gain traction and to see iterations on the idea. I think the concept will make responsive design much easier to work with. I myself borrowed the concept and sampled existing work from @lolmaus and others that I've mentioned in my README.

As a side note, I started with element-resize-detector. I felt it was the best approach at the time but a few months in already it seemed like ResizeObserver was gaining traction. Most of my work started even before ember-did-resize-modifier was around and now it looks like there's a ember-resize-observer-modifier which uses ResizeObsever.

It's true my addon has quite a few pieces that were somewhat needed in the early days. My addon took some interesting design changes and evolved pretty quickly along side the prep for my talk (the first implementation used custom Computed Properties and stashed state like ember-concurrency -- I couldn't explain it today if I had to). It currently allows for swapping different resize methods (somewhat highlighted in this cryptic note to myself #2), but in reality I think I should probably just drop the unnecessary abstraction. Hopefully I'd be able to drop my custom modifier bits, too in favour of ember-resize-observer-modifier.

All in all, it's really cool to see people putting thoughts into this space. I love your demo app and hope others check out your addon (I'll add a comment to my README so others are aware). I'd definitely be interested in comparing notes, I'll hit you up on discord.

[ijlee2]

@chadian Thanks, your words mean a lot to me!

[lolmaus]

@chadian @ijlee2 Please keep me involved in the conversation!

Regarding the subject of this issue. I didn't know about ember-resize-observer-modifier, I think the issue should be resolved by leveraging it.