Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix the statement which is getting changed by Microbundle #1156

Merged
merged 4 commits into from
Feb 27, 2023

Conversation

Code-Crash
Copy link
Contributor

While testing the cross-origin iframe feature, I realized that with the compressed version, it's not working and without compression, it's working, after digging into it, I found that, the microbundle is not converting a single line statement properly, please see the attached screenshot.

I will also add this issue to the microbundle issue list as well, however, I think it should be good if we could change the statement properly, or else anyway it will crash, and catch will handle it for cross-origin iframes.

We can see in the below image, window.parent.document is there as a normal statement, which gets trimmed somehow as window in the next compressed version of the image.

uncompress

We can see here, it's getting replaced as window only, which is causing cross-origin iframe not able to work as expected.

compress

So, if we use this as a proper statement as per this pull request, it's working fine, please see if we can merge it @Juice10

@changeset-bot
Copy link

changeset-bot bot commented Feb 27, 2023

🦋 Changeset detected

Latest commit: eefc0f0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
rrweb Patch
rrweb-snapshot Patch
rrdom Patch
rrdom-nodejs Patch
rrweb-player Patch
@rrweb/types Patch
@rrweb/web-extension Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Juice10
Copy link
Contributor

Juice10 commented Feb 27, 2023

Good catch @Code-Crash!
could you run prettier on packages/rrweb/src/record/index.ts and .changeset/chatty-cherries-train.md ? Then I can merge it in

@Code-Crash
Copy link
Contributor Author

Sure, thanks.

Copy link
Contributor

@Juice10 Juice10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Code-Crash

@Juice10 Juice10 merged commit e65465e into rrweb-io:master Feb 27, 2023
@Code-Crash
Copy link
Contributor Author

FYI: @Juice10

In Microbundle, it's expected behavior, please see the thread : developit/microbundle#1029

@Juice10
Copy link
Contributor

Juice10 commented Mar 1, 2023

Thanks for sharing @Code-Crash

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants