Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

afterAttach: HTMLElement (document.getElementById) is null #841

Closed
sspilleman opened this issue Apr 11, 2020 · 11 comments
Closed

afterAttach: HTMLElement (document.getElementById) is null #841

sspilleman opened this issue Apr 11, 2020 · 11 comments
Assignees
Labels
Bug Something isn't working
Milestone

Comments

@sspilleman
Copy link

🐛 Bug Report

In my afterAttach function Im trying to get an HTMLElement(ById). It is empty so it seems the afterAttach function is fired before the viewModel is (fully) created.

🤔 Expected Behavior

I would expect that if I execute:

let element = document.getElementById('g2chart');

as my first statement in the afterAttach function that element is not null

😯 Current Behavior

element is null

💁 Possible Solution

Add following to to beginning of my afterAttach function (which obviously is a very dirty hack)

let element = document.getElementById('g2chart');
while (element === null) {
    console.log("im here");
    await this.sleep(100);
    element = document.getElementById('g2chart');
}

In my case it's executed once. I could as well wrap my code in a setTimeout function probably

💻 Code Sample

HTML File:

<div id="g2chart" style="background-color: white;padding: 2rem;"></div>

🌍 Your Environment

Software Version(s)
Aurelia 0.7.0-dev.202004092222
Language typescript
Browser chrome 81.0.4044.92
Bundler webpack
Operating System macOS 10.15.4
NPM/Node/Yarn NPM
@sspilleman sspilleman changed the title afterAttach: HTMLElement is null afterAttach: HTMLElement (document.getElementById) is null Apr 11, 2020
@sspilleman
Copy link
Author

Strange... Am I the only one having this issue?

@EisenbergEffect
Copy link
Contributor

Is your component using shadow dom? If so, none of its view's elements will be visible by querying the document.

If you need a reference to an element within your view, the best way is to use a ref attribute set to the name of a property on your view model to store the reference in. This will be more efficient since the templating engine already has references to all the elements and doesn't need to perform a separate query.

Let me know if this helps. If not, I'll need some more info since I may not be understanding the scenario exactly.

@sspilleman
Copy link
Author

sspilleman commented Apr 15, 2020

Hi Rob,

No my component is not using shadow dom (as fas as I know, how could I check this to rule this out?). I'll try your ref suggestion to see if that is available at the begin. It might not work because I believe the constructor of the charting lib actually requires the id of the element, which then might still throw an exception that the element doen't exist... I'll update when I have the chance to test this.

Tx 4 the reply!

@fkleuver
Copy link
Member

how could I check this to rule this out?

I'd say your workaround already ruled that out, because that wouldn't work either if shadow dom was the issue.

it seems the afterAttach function is fired before the viewModel is (fully) created.

That should not be possible. afterAttach is the very last thing that's fired after everything is created, mounted, etc.

Could you show the rest of the view where that element is located in the html, as well as the view model code where the charting lib is initialized?

Another thing you can try is inject Element into your view model and try to query the element from there. If that works, that would mean the app root is simply not yet attached to the document for whatever reason. But that also shouldn't happen without some code or library deliberately doing that..

@sspilleman
Copy link
Author

Hi @fkleuver ,

The contents of my view (g2.html):

<div class="g2">
    <div id="g2chart"></div>
</div>

My entire code (g2.ts)

import { inject } from "aurelia";
import { CovidServiceBackend } from '../../services/backend/covid-service';
import { Chart, registerAnimation } from '@antv/g2';
import * as interfaces from "./interfaces";
import * as utils from "./utils";
import * as animations from "./animations";

@inject(CovidServiceBackend)
export class G2 {

    chart: Chart;
    data: interfaces.IDataset;
    timeline;
    index: number = 0;

    constructor(private covid: CovidServiceBackend) {
        registerAnimation('label-appear', animations.labelAppear);
        registerAnimation('label-update', animations.labelUpdate);
    }

    createChart(index: number) {
        const animationDuration = 600;
        this.chart = new Chart({
            container: 'g2chart',
            autoFit: true,
            height: 800,
            padding: [0, 60, 0, 150], // t r b l
            theme: ''
        });
        this.chart.data(utils.handleData(Object.values(this.data)[index]));
        this.chart.coordinate('rect').transpose();
        this.chart.legend(false);
        this.chart.tooltip(false);
        this.chart.axis('country', {
            animateOption: { update: { duration: animationDuration, easing: 'easeLinear' } },
            label: { style: { fontWeight: 'bold' } }
        });
        this.chart.annotation().text({
            position: ['100%', '90%'],
            content: Object.keys(this.data)[index],
            style: {
                fontSize: 40,
                fontWeight: 'bold',
                fill: '#aaa',
                textAlign: 'end'
            },
            animate: false,
        });
        this.chart
            .interval()
            .position('country*value')
            .color('country')
            .label('value', (value) => {
                return {
                    animate: {
                        appear: {
                            animation: 'label-appear',
                            delay: 0,
                            duration: animationDuration,
                            easing: 'easeLinear'
                        },
                        update: {
                            animation: 'label-update',
                            duration: animationDuration,
                            easing: 'easeLinear'
                        }
                    },
                    offset: 5,
                    style: { fontSize: 12, }
                };
            })
            .animate({
                appear: {
                    duration: animationDuration,
                    easing: 'easeLinear'
                },
                update: {
                    duration: animationDuration,
                    easing: 'easeLinear'
                }
            });
        this.chart.render();
    }

    updateChart(index: number) {
        if (!this.chart) return this.createChart(index);
        this.chart.annotation().clear(true);
        this.chart.annotation().text({
            position: ['100%', '90%'],
            content: Object.keys(this.data)[index],
            style: {
                fontSize: 40,
                fontWeight: 'bold',
                fill: '#aaa',
                textAlign: 'end'
            },
            animate: false,
        });
        this.chart.changeData(utils.handleData(Object.values(this.data)[index]));
    }

    async afterAttach() {
        let element = document.getElementById('g2chart');
        while (element === null) {
            await utils.sleep(100);
            element = document.getElementById('g2chart');
        }
        this.timeline = await this.covid.get("timeline");
        console.log("source", this.timeline);
        this.data = utils.transformTimeLine(this.timeline, 40).cases;
        console.log("data", Object.keys(this.data));
        let interval;
        const countUp = () => {
            this.updateChart(this.index)
            ++this.index;
            if (this.index === Object.keys(this.data).length) {
                clearInterval(interval);
            }
        }
        countUp();
        interval = setInterval(countUp, 1500);
    }
}

@sspilleman
Copy link
Author

sspilleman commented Apr 15, 2020

small update

If I change the tag to :

<div id="g2chart" ref="sander"></div>

and the beginning of my afterAttach function is now:

console.log(this.sander);
let element = document.getElementById('g2chart');
while (element === null) {
    console.log("sleep");
    await utils.sleep(100);
    element = document.getElementById('g2chart');
}

"sander" is indeed initialised with the HTMLElement... BUT it also still prints "sleep" to the console because element is still "null" the first time.
image
Verrrryy weird right?

@fkleuver
Copy link
Member

"sander" is indeed initialised with the HTMLElement... BUT it also still prints "sleep" to the console because element is still "null" the first time.

That confirms what I was suspecting, namely that for some reason your component is not yet attached to the document itself. Without a repro, I can't tell you why that is or how to fix it.

That aside, in general I would recommend against directly querying the document itself. Keep things local to your component, by using ref or by querying the injected Element for better performance, testability, encapsulation, etc.

The injected Element approach is like so:

export class MyVM {
  constructor(private el: Element) {}

  afterAttach() {
    const g2chart = this.el.getElementById('g2chart');
  }
}

So those would be 2 ways to address this issue, although it would be preferable to get to the bottom of why the component root is not attached to the document. Could you perhaps provide a minimal repro on github?

@sspilleman
Copy link
Author

Here ya go!

git clone git@github.com:sspilleman/afterAttached.git, npm install, npm run start

Server will be @PORT 3000

I already found "something"

  • If the app starts for the fist time (url = localhost:300, router will redirect) result is OK
  • If the url = localhost:3000/#/welcome and you navigate to localhost:3000/#/g2 result is OK
  • If the url = localhost:3000/#/g2 and refresh the browser (what happens by brwosersync while you are developing) result is NOK

So.....this may indicate that this issue is router related?

@fkleuver
Copy link
Member

fkleuver commented Apr 17, 2020

Thanks for the repro + instructions, that made it super easy to figure out what's wrong :)

So I already eluded to this on Discord, but for completeness sake (and future reference), here's the technical side of what happens:

During user-invoked navigation

  1. Aurelia starts and loads the app root in conjunction with the global lifecycle hooks
  2. The router starts from the global runBeforeAttach hook
  3. Router loads welcome component and invokes its hooks in isolation. welcome is compiled, bound and attached. (if you had afterAttach inside this component and tried to query something inside it from the document, you would get the same issue)
  4. attach lifecycle invoked on app root -> app is now available in document
  5. navigate to g2 -> router loads and invokes component lifecycles in isolation -> during afterAttach, the component is available in document since app root already was

During direct navigation (/ refresh)

  1. Aurelia starts and loads the app root in conjunction with the global lifecycle hooks
  2. The router starts from the global runBeforeAttach hook
  3. Router loads g2 component and invokes its hooks in isolation. g2 is compiled, bound and attached. During afterAttach, the app root is not yet attached to the document. this is where you get the null element
  4. attach lifecycle invoked on app root -> app is now available in document

In summary, au-viewport currently doesn't 'connect' the lifecycles of the component it's supposed to load to the lifecycles of the component 'hosting' it (in this case the app root), so the whole lifecycle from start to finish happens right in-between bind and attach of the host component.

The question is whether we want for example compile and bind to be wired up the same way (this would be difficult with the way the router currently relies on the bind lifecycle to happen before it can even determine what component to load), the answer is probably not. But attach should certainly be wired up in this case.
The fix for that may also require some specific tweaks in the runtime to make wiring up a bit easier because afterAttach also still needs to support async, so I will take this fix as part of a larger refactor that I'm currently working on. It may be a week or two before that's finished, maybe a bit more maybe a bit less.

I'll let you know when it's fixed. In the meantime a simple single macroTask should be plenty of time to lift your g2's afterAttach over the root startup sequence:

    async afterAttach() {
        await new Promise(resolve => setTimeout(resolve));
        console.log(this.sander);
        let element = document.getElementById('g2chart');
    }

FYI @jwx (this is part of that larger wireup picture I was talking about)

@fkleuver fkleuver self-assigned this Apr 17, 2020
@fkleuver fkleuver added the Bug Something isn't working label Apr 17, 2020
@fkleuver fkleuver added this to the Au2 v0.7.0 milestone Apr 17, 2020
@sspilleman
Copy link
Author

Thank you @fkleuver for the perfect explanation. As there is a (dirty) workaround available this isn't blocking at all, so no worries. Looking forward to the next release! V2 is quite usable for me already anyway and would have no issues putting this "in production".

@fkleuver fkleuver modified the milestones: Au2 v0.7.0, Au2 v0.8.0 May 12, 2020
@fkleuver fkleuver modified the milestones: v0.8.0, v2.0-alpha Jul 17, 2020
@fkleuver
Copy link
Member

fkleuver commented Feb 1, 2021

This is fixed

@fkleuver fkleuver closed this as completed Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants