Iframe Removal Monitoring not Working with Custom Elements about penpal

[Aaronius]

Thanks for finding and reporting this @lpellegr! I saw your two pull requests and I'll dig deeper into them ASAP. Regarding being able to configure iframe removal monitoring, could you explain the use case for that? Thanks again.

[lpellegr]

Thanks for your quick reply.

I have 2 use cases: the first is about disabling iframe removal monitoring completely since I destroy connections properly and the window that loads the app does not aim to remain open a long time. The second is about using Penpal in a third-party widget. In that case, I prefer to keep the feature enabled but a check every hour is more than enough.

[Aaronius]

@lpellegr Is the current monitoring causing issues, are you worried about performance, or something else? It's a very inexpensive check, so I don't see performance being a concern.

[Aaronius]

Hey, I haven't tested this yet and I'm having a bit of difficulty finding understanding this from documentation, but you may know the answer. If the iframe is added to the web component but the web component is not added to the window's document, will iframe.isConnected return true or false? If it returns true, I don't think that's a good thing, because that means Penpal won't close the connection automatically and the iframe will be stuck in memory.

[lpellegr]

@Aaronius The answer to your question is false. If the iframe is added to the Web component but this last is not attached to the window DOM tree, then the iframe is not connected. The same is true if the Web component is attached and then detached. Here is a simple example to illustrate both cases (uncomment the 2 lines to test the second case):

https://jsfiddle.net/lpellegr/qxupLb0y/87/

Regarding the iframe removal monitoring option it's more about having control over what happens. I agree with you the performance hit for such a check made every minute is not critical but when embedded inside third-party pages by other developers, it becomes very hard to argue and you may lose trust when it is noticed you have code running periodically that is not required or could run much less often. Besides, in the case of an issue with Web components like pointed or whatever else involving iframe removal monitoring, you can simply disable the feature as a temporary workaround.

The PR does not change the default behavior and the changes are minimal. Although I think that could benefit other users of the library, I understand if that's something you prefer not to add. In that case, I will use a fork.

[Aaronius]

Thanks for setting up that test and explaining your reasoning @lpellegr. Thanks for the PRs as well. I'm going decline the monitoring configuration one until I see a more compelling reason to make it configurable. I'll do my best to get the other two merged and released tonight. Thanks!

[Aaronius]

The iframe.isConnected fix was published in v5.3.0. Thanks @lpellegr!