fs-webcomponents / fs-elements

Collection of FamilySearch Polymer elements.
https://www.webcomponents.org/collection/fs-webcomponents/fs-elements
MIT License
4 stars 0 forks source link

Upgrade to Polymer 2.0 #5

Closed justincy closed 7 years ago

justincy commented 7 years ago

Polymer 2.0 RC was released this week. Lets work on upgrading the components to 2.0.

justincy commented 7 years ago

I think we'll start with fs-signin and then fs-person-portrait and it's dependencies. Everything should be pretty straightforward after that.

justincy commented 7 years ago

I have fs-signin, fs-client, and fs-behavior upgraded to Polymer 2. I'm going for legacy mode which lets us continue using the 1.0 API with minimal changes; only changes I've done are related to CSS shims. All changes are being committed to polymer-2 branches in each repo.

justincy commented 7 years ago

While working on the upgrade for fs-person-families I hit a problem with timing of lifecycle events. In Polymer 1.3+ property observers were guaranteed to not fire until after ready(). That assumption is currently broken in Polymer 2 hybrid mode and is breaking dynamically added elements because they're trying to fetch data from the API before their embedded fs-client instance is setup so undefined errors are being thrown.

justincy commented 7 years ago

This jsbin seems to say that the timing I relied on wasn't guaranteed: https://jsbin.com/bewaquwugo/edit?html,console,output I'm seeing observers being called before ready() in 1.x

justincy commented 7 years ago

Turns out the issue isn't with timing of property observers with relation to ready() being called. The problem is due to the order of parents and children initialization changing. In 1.x the ready() methods of all children were called before parents but in v2 that isn't happening.

justincy commented 7 years ago

But I just proved that's not true either: v1 https://jsbin.com/tijuseyegu/edit?html,console,output v2 http://jsbin.com/nisikamade/edit?html,console,output

justincy commented 7 years ago

I need to create a minimal reproduceable test case.

justincy commented 7 years ago

The problem can be fixed (circumvented) by wrapping the API request in fs-person-portrait in a setTimeout to defer until the next tick. But I still don't understand why that's necessary when being use in fs-family and not anywhere else. In fs-family the fs-client elements are not ready until after their parents are and I can't replicate that behavior anywhere else.

I tried commenting out paper-card and leaving it's contents but the issue was still manifest which suggests it's not due to the light-dom timing changes in Polymer 2.

I see the right order of events inside fs-family if I comment out both paper-card and it's contents and instead add a single fs-person-chip.

<fs-person-chip person-id="KWWC-RCL"></fs-person-chip>

That suggests it's something with the complex configuration of fs-family.

I tried changing it from a hard-coded person-id to a binding on the person object, as is done normally in fs-family and the issue returned.

<fs-person-chip person="[[family.father]]"></fs-person-chip>

That narrows it down significantly. Something with the timing of property observers and bindings. I still don't understand how that affects the timing of fs-ready callbacks (including those that aren't immediate children) but I'm getting a lot closer.

justincy commented 7 years ago

🎉 I finally created a reproducable test case in JS Bin: http://jsbin.com/xexuturave/edit?html,console,output

justincy commented 7 years ago

Reported the issue to Polymer at https://github.com/Polymer/polymer/issues/4447

justincy commented 7 years ago

The bug was confirmed by Polymer. There's not much to do until that's fixed.

justincy commented 7 years ago

Polymer released a fix for the bug. I confirmed that it works. We're back in business.

justincy commented 7 years ago

I hit an issue when upgrading the sample app. Polymer 2 doesn't guarantee any order of events for children and that's causing the client to not be configured properly. In v1 we relied on the order being guaranteed so we put the fs-client with configuration at the beginning and the client was correctly setup. But now that order isn't guaranteed, it's not necessarily being setup first so a non-configured client is being setup first and it creates the SDK instance with no config properties. Then when the configured fs-client is setup it's properties are ignored because the SDK instance is already setup. So either we need to find a way and rely on ordering or update the SDK to allow config properties to be changed after instantiation.

justincy commented 7 years ago

Now that https://github.com/FamilySearch/fs-js-lite/issues/28 added the config() method to the SDK we can stop relying on timing and just setup the SDK instance as we go.

justincy commented 7 years ago

I updated fs-client so that it works regardless of order.

Next I need to address this issue as reported in the console:

paper-toolbar is deprecated. Please use app-layout instead!

justincy commented 7 years ago

There's one last issue. The person history dropdown is not updating properly. The dropdown lists updates as we browse but the label that shows the currently selected value when it's collapsed doesn't update properly.

After hours of investigation I believe it's caused by the DOM elements being recycled when the list changes. This causes observers to not detect changes (because the selected DOM element didn't change, just the contents did) and thus change events are not emitted and the dropdown menu doesn't get notified that the list changed.

justincy commented 7 years ago

Related: https://github.com/Polymer/polymer/pull/4363

justincy commented 7 years ago

Bug report: https://github.com/PolymerElements/iron-selector/issues/154

justincy commented 7 years ago

After figuring out exactly what the bug was, it was pretty easy to find a work around. So now we're all done with the v2 upgrade. 🎉