-
Notifications
You must be signed in to change notification settings - Fork 238
90 version compat #91
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
base: master
Are you sure you want to change the base?
Conversation
a8145a4 to
c5caec2
Compare
|
@indianburger can you review this and see if it fixes your issues? |
|
@indianburger I need to write some unit-tests, but don't want to block you 😃 I've added a pretty detailed comment above the new conflict resolution code. It's late and I'm tired, so I could have easily missed something. |
|
@indianburger what's your status? Have you found a workaround or are you still interested in this fix? |
| var compatible = version === FastDom.prototype.version; | ||
| var exports = compatible ? existing : new FastDom(); | ||
| if (compatible) return existing; | ||
| console.warn('[fastdom] Multiple incompatible versions detected (this could impact performance)'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about adding a link to the issue in the warning?
|
Looks good and works for us. 👍🏾 |
|
Also to note, we hacked a version together to unblock us temporarily. So feel free to take a bit more time to add tests and do it proper. |
|
Cool. I'm on vacation for the next week. I'll be able to take a look next week. |
|
Start |
|
still on vacation? @wilsonpage |
No description provided.