search

webcomponents / polyfills

Web Components Polyfills
BSD 3-Clause "New" or "Revised" License
1.14k stars 165 forks source link

[ShadyDOM] shouldFlush is activated too late #62

Open attilapuskas opened 6 years ago

attilapuskas commented 6 years ago

It could happen that the following flusher invocation https://github.com/webcomponents/webcomponentsjs/blob/ef20def3999ca670f4141be97958dea086ef57cb/entrypoints/webcomponents-bundle-index.js#L38 resulted in another internal flush invocation which will be broken due to the disabled shouldFlush flag.

(For example vaadin-overlay triggers another flush from after a templateChange in https://github.com/vaadin/vaadin-overlay/blob/ed9a39e261bc20c5548f633f63e94e4fa93f8107/src/vaadin-overlay.html#L134)

It can be easily fixed if we move the shouldFlush = true; line above the flusher call. https://github.com/webcomponents/webcomponentsjs/blob/ef20def3999ca670f4141be97958dea086ef57cb/entrypoints/webcomponents-bundle-index.js#L39

TimvdLippe commented 6 years ago

I am not 100% sure if I follow. Could you create a JSBin that shows the issue?

attilapuskas commented 6 years ago

Sure, here it is https://jsbin.com/kuvapagopu/edit?html,js,console,output

As you can see it produces an error: Cannot read property 'length' of undefined inside the webcomponents-bundle.

If the webcomponents-bundle-index.js set the shouldFlush=true before calling the flusher then this error would be fixed. As I saw the webcomponents-loader.js has already working like that so that one does not need this fix.

TimvdLippe commented 6 years ago

@kenyerr That seems like a reasonable fix. Do you mind opening a PR here? You can use https://github.com/webcomponents/webcomponentsjs/blob/master/tests/bundle-after-load.html as a base for creating the corresponding test. Let me know if you are running into any issues and we can help you out :smile:

TimvdLippe commented 6 years ago

@kenyerr Were you able to figure it out or do you need some help with contributing? (Happy to help you out!)

attilapuskas commented 6 years ago

Hi, Sorry I had no time for that and I am afraid I won't have in the near future. So if you want to you can start to have a look at it. Of course I can do it as well but likely to be later than sooner. Thanks!

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.