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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Handle multiple sourceless nodes #1545

Merged
merged 3 commits into from Jan 27, 2023
Merged

fix: Handle multiple sourceless nodes #1545

merged 3 commits into from Jan 27, 2023

Conversation

arthurfiorette
Copy link
Contributor

@arthurfiorette arthurfiorette commented Jan 26, 2023

Hello @domoritz, it's me again 馃ぁ馃憤

At #1386 I let a todo to resolve later because I didn't think that it would impact on anything.

const file = !source
? // TODO: Use better filename for unknown files (See #1386)
"unresolved"

Seems out that we can have conflicts with sourceless nodes for objects with deep properties. And by conflicts I mean different types having the same ID, overriding each other when joining definitions.

This PR introduces the fix and a very deep test case for it.

P.S. Would you release as soon as you can? I have two a critical issue waiting for this 馃榿

Version

Published prerelease version: v1.2.1-next.1

Changelog

馃帀 This release contains work from a new contributor! 馃帀

Thank you, Grant Dickinson (@grant-d), for all your work!

馃悰 Bug Fix

馃敥 Dependency Updates

Authors: 4

@arthurfiorette
Copy link
Contributor Author

Coverage reduction was here. Probably only the previous "unresolved" string was getting there.

image

@domoritz domoritz merged commit 8d95b0a into vega:next Jan 27, 2023
@domoritz
Copy link
Member

Thank you

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.

None yet

2 participants