Aaronius / penpal

A promise-based library for securely communicating with iframes via postMessage.
MIT License
381 stars 56 forks source link

Option to skip iframe validation? #60

Closed markerikson closed 3 years ago

markerikson commented 3 years ago

I'm using Penpal in an app, and I'm trying to test that app via Cypress ( https://www.cypress.io/ ).

Unfortunately, Cypress is finicky about dealing with iframes. Part of that is that it appears to modify the value of window.top in order to host your app in its test environment, or something along those lines.

Penpal is currently doing an iframe validation check:

https://github.com/Aaronius/penpal/blob/26f493d2863dd49422efa664b58c24c5e5d1869b/src/child/validateWindowIsIframe.ts#L5

which is being called unconditionally:

https://github.com/Aaronius/penpal/blob/26f493d2863dd49422efa664b58c24c5e5d1869b/src/child/connectToParent.ts#L59

It seems like Cypress's experimentalSourceRewriting option successfully modifies Penpal enough to let the test continue, but that rewriting process takes a very long time to run as the test starts.

I can confirm that if I hand-edit the contents of node_modules/penpal to comment out the validateIframe() check, that my app loads okay in Cypress.

Would it be possible to get an option added to connectToParent() to allow skipping that check? It looks like all that's needed is adding a boolean to the Options type and then moving the validate check into an if statement.

Aaronius commented 3 years ago

Hey @markerikson, thanks for logging the issue. Will you try (or have you tried) setting modifyObstructiveCode to false in your cypress.json and see if that fixes your issue? We're using Penpal + Cypress on a project at work and I think this is what fixed it in our case.

markerikson commented 3 years ago

Hmm. Here's what I was trying in the process of getting stuff to work earlier today:

So, I never technically tried "modifyObstructiveCode": false, but that's because it wasn't on in the first place and I assume that false is the default.

Any further thoughts?

Aaronius commented 3 years ago

modifyObstructiveCode is true by default. Try setting it to false and let me know how it goes please.

markerikson commented 3 years ago

Huh. Clearly I misread that docs page :) It does indeed say it defaults to true.

Yeah, I just tried uncommenting those lines in node_modules, set "modifyObstructiveCode": false in my cypress.json, and rebuilt the app, and the tests (still) work.

Might still be useful to have this as an option anyway, I guess, but doesn't appear to be necessary to solve this use case now.

Thanks for helping clear that up! Feel free to close this if you don't plan to make the change.

Aaronius commented 3 years ago

I'm going to update the readme with some of these details, then probably create a new ticket to just remove the check entirely in the next major version since it probably doesn't provide a lot of value anyway.

Aaronius commented 3 years ago

I've removed the check from Penpal. This change was released in v6. Thanks!