Regression after merging #73

[alshakero]

This is related to Palma project.

There is a huge patch coming, that it is followed by smaller patches that are causing issues.

You'd think there is some sort of a race condition and the smaller patches assume a certain state the doesn't exist at this point and that's why the cause an exception when they're attempted to be applied. Which is not the case.

1. When I delay the applying of the smaller following patches even for 30 seconds the exception is still thrown.
2. When I delay the applying of the bigger patch itself, the smaller patches don't come! It's like the appliance of the bigger patch triggers the request for the smaller following ones, which should diminish the race condition. But once I apply the big one, the following small ones come and an exception occurs.

I made sure that their app doesn't have the root cause by removing the panic mode (explained in #73), and it worked.

After a suggestion from @warpech I checked if the patches are same with and without the panic mode enabled, and there is a difference:

Reproduction steps

1. Go to URL†.
2. Change this drop down value to "Art".

Expected outcome

Nothing, no UI changes are expected, but no console error too.

Actual outcome

Nothing, no UI changes are happening, an error is thrown in the console.

† ask for the URL and creds on Slack.

Setting up your environment

1. Download Fiddler if you don't have it.
2. Do this
3. Then this:

Now you can modify the local palindrom-client file and see the changes.

[alshakero]

/cc @tomalec

[warpech]

I've spent some time investigating it and probably made the same full circle as @alshakero, boiling down the problem to something similar to this: https://github.com/Polymer/polymer/issues/4279

Which is a notification problem fixed in Polymer 2.

---

@alshakero perhaps it is worth trying a different approach of doing the panic mode? Instead of https://github.com/Palindrom/palindrom-client/blob/969c0f3bcf3b0cc4786aecc916d7cec985b9b691/palindrom-client.html#L234-L240, you can try something that I hacked as:

var newElem = document.createElement(this.bound.localName, this.bound.getAttribute("is"));
newElem.innerHTML = '<starcounter-include view-model="{{model}}"></starcounter-include>';
nnewElem.id = this.bound.id;

var parent = this.bound.parentNode;
parent.removeChild(this.bound);
parent.appendChild(newElem);

this.bound = newElem;
this.bound.model = this.obj;

The idea here is to make a clone of the bound element as it looked before binding, remove the bound element and attach a fresh bound element. It worked well for me (no error in topic!), except that a middle part of the current page was not re-rendered. But maybe that's due to another bug?

[alshakero]

I achieved the same outcome of this with a simpler solution, but I couldn't investigate (the empty middle area issue) deeper because it's tremendously hard to know which file contains which code, then add a Fiddler rule for it, and then play with it.

I then got what seemed like a great idea, to find the longest common string among all patches, which refers to the root object that needs to be updated. I managed to get this path nicely in O(N) but when I do domBind.set(foundPath, domBind.get(foundPath)) it is considered a write operation and the server rejects it. And when I do notifyPath(foundPath) nothing happens. The notifyPath solution would work under Polymer 2.0.

I think at this point I need access to the server code to at least modify client-side code and see if the missing middle area is a bug.

[warpech]

I achieved the same outcome of this with a simpler solution, but I couldn't investigate (the empty middle area issue) deeper because it's tremendously hard to know which file contains which code, then add a Fiddler rule for it, and then play with it.

I find it super easy to locate any (unminified) code file using Chrome Network tab search tool. See here, searching for a URL of a HTML view that contains the word "example":

Is your case more complex?

[alshakero]

Is your case more complex?

A little more. This whole part of the UI is coming via JSON.

But I didn't know about this search feature. I'll find the template HTML using "inspect element" and search for some keyword from the HTML code.

[alshakero]

I achieved the same outcome of this with a simpler solution

Here is the code for the simpler solution I mentioned above if you want to play with it

```html ```

---

I found out what's happening. There is a deep dom-repeat that populates a table that fills the missing part of the UI. This dom-repeat is not being re-rendereed with this fix, causing an empty UI. notifySplice-ing this dom-repeat manually fixes this part of the issue. But there are other places in the UI that also need re-rendering and even notifying this dom-repeat alone is impossible.

Also, the current state of the code is better than this suggesion, because the same dom-repeat re-renders just fine with it but other issues emerge. So it's safe to say the suggesed solution fails at earlier stage than the current code.

However, even though current code passes this limit, it's not better. Many things are breaking badly after a huge patch is applied. I could track each. But I doesn't seem rational to fix several issues (assuming they're fixable) to serve a workaround.

I suggest we rollback the fix and suggest migrating to Polymer 2.0 to get a better performance. All these fixes and time we're spending will be trashed anyways when they migrate to Polymer 2.0.

[warpech]

However, even though current code passes this limit, it's not better. Many things are breaking badly after a huge patch is applied. I could track each. But I doesn't seem rational to fix several issues (assuming they're fixable) to serve a workaround.

I suggest we rollback the fix and suggest migrating to Polymer 2.0 to get a better performance. All these fixes and time we're spending will be trashed anyways when they migrate to Polymer 2.0.

I agree. Please discuss with Erik.

[warpech]

Closing as won't fix. It was confirmed with Jimmy that they will look into migrating to Polymer 2 instead of working around Polymer 1. See https://starcounter.slack.com/archives/C67JPA7NG/p1531815185000187