miguelcobain / ember-composability-tools

ember-composability-tools - Helpers for building a somewhat different kind of components.
MIT License
39 stars 18 forks source link

Make the addon promise aware #58

Closed jagthedrummer closed 4 months ago

jagthedrummer commented 4 months ago

This makes it so that asyncronous setup in a parent component will be awaited before trying to setup child components.

If any given didInsertNode/didInsertParent functions are NOT marked with async then I think this change should have no effect on them.

Fixes #57

Example usage:

test/parent.hbs

<div
  {{!--
  These two actions are inherited from Root, but we need to call them
  manually since we're not using the Root template.
  --}}
  {{did-insert this.didInsertNode}}
  {{will-destroy this.willDestroyNode}}
  ...attributes
>
  <div>The parent</div>
  {{yield (component "Test/Child" parent=this)}}
</div>

test/parent.js

import { Root } from 'ember-composability-tools';

export default class TestParentComponent extends Root {
  @tracked isReady = false;

  // this doesn't need to be an action since it's called directly by Root
  async didInsertParent(element) {
    console.log('Parent: didInsertParent', this.args.id);
    // This simulates an async setup process
    await timeout(this.args.setupTime || 0);
    this.isReady = true;
    console.log('Parent: finally ready', this.args.id);
  }
}

test/child.hbs

<div>The child</div>

test/child.js

import { Node } from 'ember-composability-tools';

export default class TestChildComponent extends Node {
  parentIsReady(){
    console.log('Child: parent is ready', this.args.id);
  }

  // this doesn't need to be an action since it's called directly by Node
  didInsertParent(element) {
    console.log('Child: didInsertParent', this.args.id, this.args.parent.isReady);
  }
}

test.hbs

<Test::Parent @id="p1" @setupTime={{1000}} as |Test::Child|>
  <Test::Child @id="c1.1"/>
  <Test::Child @id="c1.2"/>
</Test::Parent>

<Test::Parent @id="p2" @setupTime={{100}} as |Test::Child|>
  <Test::Child @id="c2.1"/>
  <Test::Child @id="c2.2"/>
</Test::Parent>

Console output with this PR:

// immediately
Parent: didInsertParent p1
Parent: didInsertParent p2

// after 100 ms
Parent: finally ready p2
Child: didInsertParent c2.1
Child: parent is ready c2.1
Child: didInsertParent c2.2
Child: parent is ready c2.2

// after 1000 ms
Parent: finally ready p1
Child: didInsertParent c1.1
Child: parent is ready c1.1
Child: didInsertParent c1.2
Child: parent is ready c1.2

Console output without this PR:

// immediately
Parent: didInsertParent p1
Child: didInsertParent c1.1
Child: didInsertParent c1.2
Parent: didInsertParent p2
Child: didInsertParent c2.1
Child: didInsertParent c2.2

// after 100 ms
Parent: finally ready p2

// after 1000 ms
Parent: finally ready p1
jagthedrummer commented 4 months ago

@miguelcobain, just curious if you have a feel for whether or not this is a safe change. It seems to be working for me like I'd expect. I'm almost ready to deploy a new version of my project that's using it, and I'm wondering if I'll be able to use the regular add-on of if I'll need to set things up to use my own fork.

jagthedrummer commented 4 months ago

I see that there are a couple of test failures here. One looks like it may be a legit failure related to this PR, and the other seems to be a dependency issue of some sort. I'm running into that same dependency issue locally when I try to run the test suite, both on my branch and on mater, so I'm not sure how to even investigate the other failure at the moment.

$ ember test
====================================================================

  The `ember` node module is a placeholder, you may be looking for:

  * `ember-cli` (the command line tool)
  * `ember-source` (the framework code)

  Visit https://emberjs.com/ for more details

====================================================================

  Forwarding request to ember-cli via `npx ember-cli test`
Environment: test
cleaning up...
Build Error (FastBootConfig)

The comparison function must be either a function or undefined

Stack Trace and Error Report: /var/folders/fx/vmqbtyws34j1l88m16_9jy_00000gn/T/error.dump.749d2e4a685fa1a4e525caab63bbd8fc.log

I think this is the relevant portion of that log:

  - broccoliBuilderErrorStack: TypeError: The comparison function must be either a function or undefined
    at Array.sort (<anonymous>)
    at stringify (/Users/jgreen/projects/ember-composability-tools/node_modules/json-stable-stringify/index.js:73:31)
    at stableStringify (/Users/jgreen/projects/ember-composability-tools/node_modules/json-stable-stringify/index.js:90:3)
    at FastBootConfig.toJSONString (/Users/jgreen/projects/ember-composability-tools/node_modules/ember-cli-fastboot/lib/broccoli/fastboot-config.js:170:12)
    at FastBootConfig.build (/Users/jgreen/projects/ember-composability-tools/node_modules/ember-cli-fastboot/lib/broccoli/fastboot-config.js:56:53)
    at TransformNodeWrapper.build (/Users/jgreen/projects/ember-composability-tools/node_modules/broccoli/dist/wrappers/transform-node.js:71:39)
    at /Users/jgreen/projects/ember-composability-tools/node_modules/broccoli/dist/builder.js:185:30
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Builder.build (/Users/jgreen/projects/ember-composability-tools/node_modules/broccoli/dist/builder.js:204:13)
    at async Builder.build (/Users/jgreen/projects/ember-composability-tools/node_modules/ember-cli/lib/models/builder.js:204:11)
miguelcobain commented 4 months ago

Closed in favor of #59