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

fix(ios): undefined is not an object (evaluating 'measureSpec.setColumn') #7230

Merged
merged 4 commits into from
May 31, 2019

Conversation

m-abs
Copy link
Contributor

@m-abs m-abs commented May 10, 2019

We, the rest of the NativeScript community, thank you for your
contribution!
To help the rest of the community review your change, please follow the instructions in the template.
-->

PR Checklist

What is the current behavior?

In the past month we've tracked 20+ crashes with this error:

TypeError: undefined is not an object (evaluating 'measureSpec.setColumn')

We haven't been able to consistently reproduce the crash locally, but I think it is clear where it does wrong.

What is the new behavior?

GridLayout(ios).onMeasure(...) will create MeasureSpecs if missing the for child view when it calls this.eachLayoutChild(...)

Fixes: #7226

@cla-bot cla-bot bot added the cla: yes label May 10, 2019
@ghost ghost added the ♥ community PR label May 10, 2019
@@ -136,6 +136,11 @@ export class GridLayout extends GridLayoutBase {

this.eachLayoutChild((child, last) => {
let measureSpecs = this.map.get(child);
if (!measureSpecs) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was reviewing our code and I am not sure trying to fix this exceptional situation by registering layout child here is a good idea. How about just skipping this specific layout child instead (as obviously something is not right with its current state)?

if (!measureSpecs) { 
    continue;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've finally managed to reproduce the problem see: #7226 (comment)

I'm not sure what the best solution is or if we just use components+layouts incorrectly.

Just skipping as you suggest results in the component not being rendered.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @m-abs thank you isolating the problem!
So just skipping the iteration when measureSpec is undefined is not an option, ok. Will investigate the code on the nativescript-angular side a bit but if I don't find a better solution I am going to merge yours as is.

Meanwhile, there is also a workaround that you can use -- wrap the contents of your sub1 component (sub1.component.html) in any kind of layout like this:

<GridLayout>
    <ns-sub2></ns-sub2>
</GridLayout>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m-abs I think we found the root of the problem in the ProxyViewContainer class -- see my commit here: 3acfc70

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@manoldonev manoldonev self-assigned this May 22, 2019
@manoldonev
Copy link
Contributor

test

@manoldonev manoldonev merged commit 888fc57 into NativeScript:master May 31, 2019
manoldonev added a commit that referenced this pull request Jun 3, 2019
@m-abs m-abs deleted the fix/measureSpec-is-undefined branch August 19, 2019 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash(iOS): TypeError: undefined is not an object (evaluating 'measureSpec.setColumn')
2 participants