Component Lifecycle Methods are not called in the correct order #1261

ghost · 2018-12-03T16:45:24Z

Stencil version:

 @stencil/core@0.16.0

I'm submitting a:

[x] bug report
[ ] feature request
[ ] support request => Please do not submit support requests here, use one of these channels: https://stencil-worldwide.herokuapp.com/ or https://forum.ionicframework.com/

Current behavior:

The lifecycle methods are not called in the correct order:
NOTE: I've changed CmpA (cmp-a) to CmpZ (cmp-z) which makes the bug appear more often

CmpD - a1-child client componentWillLoad
CmpD - a2-child client componentWillLoad
CmpD - a3-child client componentWillLoad
CmpD - a4-child client componentWillLoad
CmpB client componentWillLoad
CmpZ client componentWillLoad <--
CmpC client componentWillLoad
CmpD - c-child client componentWillLoad
CmpD - a1-child client componentDidLoad
CmpD - a2-child client componentDidLoad
CmpD - a3-child client componentDidLoad
CmpD - a4-child client componentDidLoad
CmpZ client componentDidLoad <--
CmpD - c-child client componentDidLoad
CmpC client componentDidLoad
CmpB client componentDidLoad

Fails in Chrome on Windows, Safari on macOS and iOS/iPhone, Chrome on Android.
IE11 and Edge has the correct hiarchy but childs are loaded reversed.

Expected behavior:

The lifecycle methods should be called in the same order on server and client.

CmpZ server componentWillLoad <--
CmpD - a1-child server componentWillLoad
CmpD - a2-child server componentWillLoad
CmpD - a3-child server componentWillLoad
CmpD - a4-child server componentWillLoad
CmpB server componentWillLoad
CmpC server componentWillLoad
CmpD - c-child server componentWillLoad
CmpD - a1-child server componentDidLoad
CmpD - a2-child server componentDidLoad
CmpD - a3-child server componentDidLoad
CmpD - a4-child server componentDidLoad
CmpD - c-child server componentDidLoad
CmpC server componentDidLoad
CmpB server componentDidLoad
CmpZ server componentDidLoad <--

Steps to reproduce:

Refresh the following page and compare the server, and client messages. They should be in the same order:
https://stencil-bug.firebaseapp.com/prerender-z/

Changing the prerender-test component "cmp-a" tag, to "cmp-z" makes the lifecycle methods to be called out of order. This is the original test:
https://stencil-bug.firebaseapp.com/prerender-a/

Related code:
PR for tests: #1266

Other information:

#1130

The text was updated successfully, but these errors were encountered:

ghost · 2018-12-05T11:57:43Z

Added PR for tests: #1266

DominicBoettger · 2018-12-19T00:40:47Z

I can confirm this issue.
It is only a problem when using prerendering. The order without prerendered HTML seems to be correct.
This creates disturbing flickering for the user as the component data is empty in some cases.

jarrvis · 2018-12-19T16:27:02Z

Also confirming this issue.
This is blocking a bit normal usage of Redux with 'slot' components because child 'slot' components can be loaded before parent which means that Redux store which is initialized in parent componentWillLoad is not there yet available in child slot componentWillLoad

DominicBoettger · 2018-12-19T16:35:05Z

We checked the current codebase and then tried to find a solution for the order.

The current queue in the core.browser does not respect the need that a child component maybe needs data from a parent component. If the order should be correct it should maybe work like the dummy code below.

@adamdbradley My colleague @DanielTWalther and me tested that topic in the last days and came to the result that it's maybe necessary to rewrite the code in the core.browser to respect the order like in the dummy code below. What do you think about the topic and do you have plans for the queue mechanism in the core.browser?
If you want we can modify the core.browser so it works like the code below in the situation the page has been prerendered.

<a-test class="hydrated">
  <b-test class="hydrated">
    <div class="mytest">
      <c-test class="hydrated">
      </c-test>
      <d-test class="hydrated">
        <e-test class="hydrated">
        </e-test>
      </d-test>
    </div>
  </b-test>
</a-test>

const bodyEl = document.querySelectorAll('body > *');


const iterateBodyWillLoad = async function(bodyEl) {
	for(const el of bodyEl) {
		if(el.classList.contains('hydrate')) {
    	// exec viewWillLoad of component
      // await for viewWillLoad function
      console.log('willLoad: ' + el.nodeName)
    }
    if(el.children) {
    	// check for children and recursively iterate
      iterateBodyWillLoad(el.children);
    }
    if(el.classList.contains('hydrate')) {
        // exec render function
		    console.log('render: '+ el.nodeName);
    }
  }
}

iterateBodyWillLoad(bodyEl);

jarrvis · 2018-12-19T16:50:24Z

Well my opinion is that it would be the best. I'm currently doing sth similar in some sense to overcome the issue: looping recursively over parent's children in parent componentDidLoad and calling forceUpdate() on every Stencil component found to have some hook in child where parent is for sure initialized. But this is not the nicest and more of a temp solution. If you are able to change it I would definitely appreciate

jarrvis · 2018-12-21T20:58:53Z

Hey, I just found acceptable (for me) solution for my issue with Redux store not being available in slot components. assuming that the root component is 'known' to any other component I can do sth like:

  componentWillLoad() {
    document.querySelector("my-root-component").componentOnReady().then(() => {
      this.store.mapDispatchToProps(this, {
        getResources
      });
      this.store.mapStateToProps(this, state => {
        return {
          loading: state.payload.loading,
          resources: state.payload.resources,
          totalNoOfResources: state.payload.totalNoOfResources,
        }
      });
    }); 
  }

ghost · 2018-12-21T22:16:37Z

Thanks for sharing - that will work for me too 😄

You can use async/await to make it cleaner:

async componentWillLoad() {
    await document.querySelector("my-root-component").componentOnReady();
    ...
}

EladBezalel · 2019-01-20T15:26:17Z

It also affects if you try to maintain components hierarchy such as

<my-b>
	<my-b>
		<my-a>
		</my-a>
	</my-b>
</my-b>

When my-a and my-b both register to their parent my-b on componentWillLoad hook

current output

my-a will load // no my-b parent
my-a did load
my-b will load
my-b will load
my-b did load
my-b did load

expected

my-b will load
my-b will load
my-a will load
my-a did load
my-b did load
my-b did load

jarrvis · 2019-01-21T08:23:21Z

maybe this will help:
https://github.com/jarrvis/stencil-component-lifecycle-test

f10l · 2019-02-23T16:55:12Z

async componentWillLoad() {
    await document.querySelector("my-root-component").componentOnReady();
    ...
}

I tried using this, but sometimes it just crashes the app and the componentOnReady() seems to never finish.

jarrvis · 2019-02-23T18:26:56Z

@fanick-ber try with:

    document.querySelector("my-root-component").componentOnReady().then(() => {
         ...
      });

I also noticed that it breaks the app when root component was actually loaded before child. Did not analyze yet why but Promise#then works always for me

jarrvis · 2019-02-23T18:38:50Z

Funny thing is that its about the alphabetical order of component names:) You can check in that test repo that I linked

f10l · 2019-02-23T22:13:19Z

Thanks, it's really tough to understand what's going on there. I nested many slots and occasionally the content of a slot is just flushed before the actual slot position. I haven't found any solution beside shadowing all components (which is a bit more work for shared styles).
My root component is alphabetically the first one, maybe that breaks it...

jgiordano-rms · 2019-04-15T20:00:38Z

When passing a conditional value to a component that has a slot, it causes the inner to dismount. I'm pretty sure its because of this. For example,

<some-component>{condition ? 'something' : 'something else'}</some-component>

When the condition changes is causes the text to dismount from the component.

Is this issue still on the radar for a fix? We can use the workaround but it's a bit of an issue having to add the workaround to every component in our component lib. Thank you!

polysign · 2019-04-18T13:03:27Z

This issue still persists in one of our recent examples:

This does not work in a consistent way. Fails every couple of times.

<component-a>
    <component-b></component-b>
</component-a>

This does work in a consistent way:

<x-component-a>
    <component-b></component-b>
</x-component-a>

Kinda annoying. Anyone has a workaround?

psmyrdek · 2019-04-23T09:23:01Z

Can confirm it's still there for 0.18.1 :/

root will load
root did load
child 3 will load
child 2 will load
child 3 will load
child 1 will load
child 3 did load
child 2 did load
child 3 did load
child 1 did load

(It works in the same way in 1.0.0 alpha-12., but I'm not sure if this issue was considered resolved with 1.0.0-x)

polysign · 2019-04-23T10:07:36Z

So we ended up doing the following (based on some replies in this thread), but not sure if it will stay and work across all major browsers etc. >>> We did not test it yet :)

In the component that fails, we added the following code in the componentWillLoad() lifecycle hook:

Element() el: HTMLElement;

async componentWillLoad() {
  if (this.el.parentElement.componentOnReady) {
    await this.el.parentElement.componentOnReady();
  }
}

It's definitely a work-around and hopefully not needed in some future releases.

psmyrdek · 2019-04-23T12:24:13Z

I was thinking that your fix is gonna help with #1439 but unfortunately it didn't :/ It seems that these are two different issues.

polysign · 2019-04-23T12:35:56Z

@ps011 we did not test this yet on IE11.
Also, this fix seems to work if there is only a single Element within the parent's slot. Not sure why, but once we have 2 or 3 of the same components in the slot of a common parent, the fix does not work anymore. We will continue investigating.

MonsieurMan · 2019-04-24T11:29:38Z

@adamdbradley Is anybody working on the issue ? It makes stencil simply unusable... 😢
Because of this, we have to put our initialization logic in the render function as we never know if the slots are actually all loaded making the component outrageously slow and sloppy.

polysign · 2019-04-24T12:11:50Z

I'll be working on something tomorrow to see if we can fix it somehow in our code. Maybe it could be ported to the core later on. I'll keep you posted.

SgtPooki · 2019-06-17T22:39:51Z

I cloned https://github.com/jarrvis/stencil-component-lifecycle-test.git and made a few minor changes with the old version.. reproduced the issue, then realized it was using a much older version of stenciljs.

I upgraded the version (see changes here: SgtPooki/stencil-component-lifecycle-test@67416a6) and the issue was not gone. Output is below:

cmp-a will load 620.9100000560284
cmp-b will load 621.4800000889227
cmp-c will load 622.0550000434741
cmp-a render 2535.0550001021475
cmp-a did load 2537.1750000631437
cmp-b render 2537.3949999921024
cmp-b did load 2537.735000019893
cmp-c render 2537.899999995716
cmp-c did load 2538.1150000030175

kraftwer1 · 2019-06-19T07:21:43Z

@manucorporat I can confirm:
This issue still exists in Stencil One. The execution order of componentWillLoad() is non deterministic.

Agree with @MonsieurMan that this bug makes Stencil One unusable for many people. Therefore sticking with 0.15.1 for the moment.

sfmskywalker · 2019-07-19T18:57:04Z

Same here. I started using Redux, and things are now broken due to the non deterministic nature of the order in which life cycle events fire. 2 out of 3 times, a child component will try to access the store which has not yet been created by the root component.

@kraftwer1 Good to know 0.15.1 still works - I'll give that a go.

sfmskywalker · 2019-07-19T19:10:13Z

Thanks for sharing - that will work for me too 😄

You can use async/await to make it cleaner:
async componentWillLoad() {
    await document.querySelector("my-root-component").componentOnReady();
    ...
}

That worked a treat! I encapsulated it in a small helper class as to not have to repeat the root component name in each child component (I have a lot of them). Now I can do:

async componentWillLoad() {
    await ComponentHelper.rootComponentReady();
   ...
}

ComponentHelper:

export class ComponentHelper {
  static rootComponentReady(){
    return document.querySelector("my-root-component").componentOnReady();
  }
}

pranjalworm · 2019-08-26T06:40:13Z

I can also confirm this issue.

robinsummerhill · 2019-10-03T14:58:11Z

This issue is still present in v1.5.4.

For:

<cmp-c>
  <cmp-b>
    <cmp-a></cmp-a>
  </cmp-b>
</cmp-c>

The 'will load'/'did load' order is incorrect and dependent on browser.

Any feedback on the status of this? It's a fundamental problem and putting me off adopting stencil.js.

robinsummerhill · 2019-10-03T19:56:27Z

Looking at the code, I can see why the order of lifecycle events is not correct. Here's the sequence of events from registering components to initial updates:

Components are registered with bootstrapLazy(...) in alphabetical order in app.esm.js generated during the build. (src/runtime/bootstrap-lazy.ts#133)
The registered custom component is an instance of HostElement (declared in src/runtime/bootstrap-lazy.ts#67). This has an s_init method that is used later on to detect whether an element is a custom Stencil element.
Immediately after registration, the connectedCallback handler of HostElement is called.
The handler walks up the DOM tree looking for an ancestor element that is a custom Stencil element (it is looking for the presence of the s_init method) (src/runtime/connected-callback.ts#65
If a Stencil ancestor is found then the child is added to a list of children for the ancestor (src/runtime/connected-callback.ts#75)
When the component is finally initialized, an update is added to the ancestor's queue, if found, otherwise the update is scheduled immediately. I assume that this is the basis of how the lifecycle events are suppose to be ordered i.e. the child won't be initialized until the ancestor has been initialized. (src/runtime/initialize-component.ts#107)

The problem stems from the fact that connectedCallback for the first registered component is called before other components have been registered. Thus, an ancestor Stencil component is never found and components will be initialized immediately without waiting for their ancestors.

I don't have enough knowledge of the codebase to know what to do about this but it looks pretty fundamental. I can only assume that the current tests pass by coincidence because the components are nested in alphabetical order.

MonsieurMan · 2019-10-04T08:16:06Z

@robinsummerhill Maybe a PR with a failing test would be a good first step ?

edit: Just saw that OP actually added one back then.

edit: I'm embarassed to do that, but @adamdbradley, @manucorporat, @jthoms1 could you give us your thoughts on this issue. Are you not going through it ? Do you use workarounds ? Is it too hard to fix and help is needed ? Thanks for your response. 🙂

robinsummerhill · 2019-10-05T08:12:15Z

Just a bit more information after looking at the existing tests and writing a new karma test for this particular issue:

The problem does not occur when the nesting is set up using the render method of each component. i.e. cmp-c renders cmp-b and cmp-b renders cmp-a. Existing tests (e.g.lifecycle-basic) cover this and pass.

The problem only occurs when the nesting is set up using nested elements in HTML and when the components are not in alphabetical order i.e.

<cmp-c>
  <cmp-b>
    <cmp-a></cmp-a>
  </cmp-b>
</cmp-c>

More worryingly, I wrote a karma test for this issue, which should fail with the current version of Stencil. It fails when run manually and observed in the browser but passes when run under karma in headless Chrome or Firefox. It appears that the order of lifecycle events is very sensitive to the environment under which the test is being run. When observed manually in the browser the order is non-deterministic and will alter with each refresh of the page.

(By the way, the reason that this is such an important issue is for cases where child components need to discover parent components or vice versa for communication - think component libraries with nested tab controls, selects with options or children retrieving a state container from a distant ancestor)

robinsummerhill · 2019-10-08T08:50:59Z

Looks like @manucorporat refactored the lifecycle code three days ago in master. This bug still exists.

arjunyel · 2019-10-08T13:57:30Z

@robinsummerhill thank you for taking a look, so you've tried it on 1.6.0-2 and its still broke?

robinsummerhill · 2019-10-08T14:01:51Z

@robinsummerhill thank you for taking a look, so you've tried it on 1.6.0-2 and its still broke?

Yes - it is broken in 1.6.0-2. The issue is very specific and not covered by any of the existing tests. Nesting needs to be set up in HTML not in the render method of a parent component and the tag names need to be in non-alphabetical order. The cause of the problem is that connectedCallback is called before all components have been registered so ancestor components are not found.

The PR above fixes the issue by deferring firing connectedCallback until after all components have been registered.

unexpand · 2019-11-21T18:40:07Z

import '@stencil/redux';
import { Store, Action } from "@stencil/redux";

this fixed the issue for me magically !

adamdbradley · 2019-12-31T18:44:22Z

Looks like it's solved now, thanks @robinsummerhill and everyone for helping to pinpoint the issue

ionitron-bot bot added the triage label Dec 3, 2018

ghost mentioned this issue Dec 5, 2018

test(prerender): test client component lifecycle order #1266

Closed

kraftwer1 mentioned this issue Jan 22, 2019

Using <slot /> ? ionic-team/stencil-redux#13

Closed

robinsummerhill mentioned this issue Oct 8, 2019

fix(lifecycles): Defer connectedCallback until all components registered #1930

Merged

adamdbradley closed this as completed Dec 31, 2019

kraftwer1 mentioned this issue Jul 27, 2020

connectedCallback()'s in wrong order #2592

Closed

Component Lifecycle Methods are not called in the correct order #1261

Component Lifecycle Methods are not called in the correct order #1261

Comments

ghost commented Dec 3, 2018 • edited by ghost Loading

ghost commented Dec 5, 2018

DominicBoettger commented Dec 19, 2018

jarrvis commented Dec 19, 2018

DominicBoettger commented Dec 19, 2018

jarrvis commented Dec 19, 2018

jarrvis commented Dec 21, 2018 • edited Loading

ghost commented Dec 21, 2018

EladBezalel commented Jan 20, 2019

jarrvis commented Jan 21, 2019

f10l commented Feb 23, 2019

jarrvis commented Feb 23, 2019

jarrvis commented Feb 23, 2019

f10l commented Feb 23, 2019

jgiordano-rms commented Apr 15, 2019

polysign commented Apr 18, 2019 • edited Loading

psmyrdek commented Apr 23, 2019 • edited Loading

polysign commented Apr 23, 2019 • edited Loading

psmyrdek commented Apr 23, 2019

polysign commented Apr 23, 2019

MonsieurMan commented Apr 24, 2019

polysign commented Apr 24, 2019

SgtPooki commented Jun 17, 2019 • edited Loading

kraftwer1 commented Jun 19, 2019

sfmskywalker commented Jul 19, 2019 • edited Loading

sfmskywalker commented Jul 19, 2019

pranjalworm commented Aug 26, 2019

robinsummerhill commented Oct 3, 2019 • edited Loading

robinsummerhill commented Oct 3, 2019 • edited Loading

MonsieurMan commented Oct 4, 2019 • edited Loading

robinsummerhill commented Oct 5, 2019 • edited Loading

robinsummerhill commented Oct 8, 2019 • edited Loading

arjunyel commented Oct 8, 2019

robinsummerhill commented Oct 8, 2019 • edited Loading

unexpand commented Nov 21, 2019

adamdbradley commented Dec 31, 2019

ghost commented Dec 3, 2018 •

edited by ghost

Loading

jarrvis commented Dec 21, 2018 •

edited

Loading

polysign commented Apr 18, 2019 •

edited

Loading

psmyrdek commented Apr 23, 2019 •

edited

Loading

polysign commented Apr 23, 2019 •

edited

Loading

SgtPooki commented Jun 17, 2019 •

edited

Loading

sfmskywalker commented Jul 19, 2019 •

edited

Loading

robinsummerhill commented Oct 3, 2019 •

edited

Loading

robinsummerhill commented Oct 3, 2019 •

edited

Loading

MonsieurMan commented Oct 4, 2019 •

edited

Loading

robinsummerhill commented Oct 5, 2019 •

edited

Loading

robinsummerhill commented Oct 8, 2019 •

edited

Loading

robinsummerhill commented Oct 8, 2019 •

edited

Loading