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

Inherit parent IDs during insertion #75

Merged
merged 4 commits into from
Apr 22, 2021

Conversation

JakeWharton
Copy link
Member

No need to be clever in the generated code. This does mean we need to switch to internal mutability, but it's a small price to pay for the generated code cleanup. Additionally, this is needed to support the forthcoming test output tree.

Refs #1. Refs #14.

We are using intermediate nodes in the internal Compose tree which previously always sent -1 as the parent ID.
This includes src/main/kotlin and src/test/kotlin but excludes src/test/fixture/counter/build/generated/../stuff.kt which is generated and does not need to conform.
@@ -37,6 +39,40 @@ import kotlin.test.assertEquals
import kotlin.test.fail

class TreehouseCompositionTest {
@Test fun childrenInheritIdFromSyntheticParent() = runTest {
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: This test is in a commit before the change to the generated code to assert that it works / does not break anything.

No need to be clever in the generated code. This does mean we need to switch to internal mutability, but it's a small price to pay for the generated code cleanup. Additionally, this is needed to support the forthcoming test output tree.
@JakeWharton JakeWharton force-pushed the jw/inherit-parent-id/2021-04-21 branch from 1492a3a to d8e61e7 Compare April 22, 2021 12:51
@saket saket self-requested a review April 22, 2021 19:24
@JakeWharton JakeWharton merged commit dae2dd0 into trunk Apr 22, 2021
@JakeWharton JakeWharton deleted the jw/inherit-parent-id/2021-04-21 branch April 22, 2021 19:56
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