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: prevent duplicative array observation patch #5654

Merged
merged 5 commits into from
Feb 24, 2022

Conversation

EisenbergEffect
Copy link
Contributor

@EisenbergEffect EisenbergEffect commented Feb 24, 2022

Pull Request

📖 Description

In the event that two copies of fast-element are loaded into the same browser context, this PR prevents doubly patching of Array methods

🎫 Issues

👩‍💻 Reviewer Notes

It's important to note that we still need to configure the array observer factory per library instance but we don't want to patch Array more than once, since that's global.

Please also review #5656

📑 Test Plan

Nearly impossible to test this. All current tests still pass.

✅ Checklist

General

  • I have included a change request file using $ yarn change
  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

⏭ Next Steps

There are likely other problems with having two instances of fast-element on the same page, particularly around looking up element definitions, observing objects built on different versions of the library, etc. We should look into whether there are ways to address these other issues as part of FASTElement 2.0. Standard decorator metadata will help with some of this eventually, but that won't land in time for FE2.0, so we can try to adopt a temporary solution with the same behavior.

@EisenbergEffect EisenbergEffect merged commit 9dfb9bb into master Feb 24, 2022
@EisenbergEffect EisenbergEffect deleted the eisenbergeffect/prevent-duplicate-array-patch branch February 24, 2022 18:38
@EisenbergEffect
Copy link
Contributor Author

@chrisdholt Can you assist with a patch release for fast-element to get this out for those who need it?

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.

fix: fast-element's Array.prototype is being patched twice resulting on duplicate splices
4 participants