Skip to content

Conversation

pjBooms
Copy link
Collaborator

@pjBooms pjBooms commented Oct 3, 2025

Compose Hot Reload Gradle plugin is bundled with Compose Gradle plugin.

Thus Compose Hot Reload is available from Compose Multiplatform projects with the desktop target.

Fixes CMP-8885

Testing

Integration tests added.

This should be tested by QA.

Release Notes

Highlights - Desktop

  • Compose Hot Reload Gradle plugin is bundled with Compose Gradle plugin (no need to configure it separately)

@pjBooms pjBooms requested a review from sellmair October 3, 2025 15:49
@pjBooms pjBooms force-pushed the pjBooms/embed-compose-hot-reload-plugin branch from 2195620 to a800d3f Compare October 6, 2025 10:11
Copy link
Member

@sellmair sellmair left a comment

Choose a reason for hiding this comment

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

Waiting for the approach with the 'preferred' dependency

Copy link
Member

@sellmair sellmair left a comment

Choose a reason for hiding this comment

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

The approach seems elegant to me.
Note: I am not reviewing the tests, but only the mechanics of how the 'bundling' works. Using a preferred versioned dependency seems like a good solution.

@pjBooms pjBooms requested a review from terrakok October 9, 2025 12:47
@pjBooms pjBooms force-pushed the pjBooms/embed-compose-hot-reload-plugin branch 5 times, most recently from 4be7a02 to 4fcf98d Compare October 13, 2025 07:58
@terrakok
Copy link
Member

terrakok commented Oct 13, 2025

topics to discuss:

  1. is it possible to apply plugin by id + version in another plugin?
  2. what will happen in multimodule project where custom CHR is applied only to one of the modules (not in root)
  3. Applied by default CHR plugin implicitly enables new code generation + new tasks require JetBrains runtime. I think we have to provide a flag to disable CHR at least. (or even not apply it by default) cc @MatkovIvan
  4. Project with no desktop target shouldn't have applied CHR

@pjBooms pjBooms force-pushed the pjBooms/embed-compose-hot-reload-plugin branch 7 times, most recently from ce75c3b to 88a3982 Compare October 16, 2025 08:19
@pjBooms
Copy link
Collaborator Author

pjBooms commented Oct 16, 2025

topics to discuss:

  1. is it possible to apply plugin by id + version in another plugin?
  2. what will happen in multimodule project where custom CHR is applied only to one of the modules (not in root)
  3. Applied by default CHR plugin implicitly enables new code generation + new tasks require JetBrains runtime. I think we have to provide a flag to disable CHR at least. (or even not apply it by default) cc @MatkovIvan
  4. Project with no desktop target shouldn't have applied CHR

"3." & "4." are addressed here.

@pjBooms pjBooms force-pushed the pjBooms/embed-compose-hot-reload-plugin branch 2 times, most recently from 05aada2 to f796ba0 Compare October 16, 2025 12:23
@pjBooms pjBooms force-pushed the pjBooms/embed-compose-hot-reload-plugin branch 3 times, most recently from 34fdad0 to 0c5274f Compare October 16, 2025 16:00
gradle("prepareKotlinIdeaImport").checks {
gradle("prepareKotlinIdeaImport", "-P$disabaleProperty=false").checks {
val expected = if (System.getProperty("os.name").lowercase().contains("windows")) {
// Windows has different line endings in comparison with Unixes,
Copy link
Member

Choose a reason for hiding this comment

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

⚠️ ⚠️ ⚠️ BTW! Does the CHR plugin break reproducible builds between Win and Unix machines?

Copy link
Member

@sellmair sellmair Oct 17, 2025

Choose a reason for hiding this comment

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

⚠️ ⚠️ ⚠️ BTW! Does the CHR plugin break reproducible builds between Win and Unix machines?

Can you please elaborate?

Copy link
Member

Choose a reason for hiding this comment

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

The only thing I could imagine is that different OS create different hashes for resources (which would be a bug?)

Copy link
Collaborator Author

@pjBooms pjBooms Oct 20, 2025

Choose a reason for hiding this comment

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

The only thing I could imagine is that different OS create different hashes for resources (which would be a bug?)

Windows has different line endings in comparison with Unixes. So vector.xml vector drawables become binary different when are they checked out from Git and thus produce different hashes.

Last time when we discussed it, we came into conclusion that the hashes must be different, because the content is different and fixed the test by providing different "expected" directories for Windows and non-Windows. However "reproducible builds" sounds like a valid contra-argument so I would handle it in the hash calculation. "Cons" of this approach was that we had to add a custom platform-specific handling logic that sounds fragile.

Copy link
Collaborator Author

@pjBooms pjBooms Oct 20, 2025

Choose a reason for hiding this comment

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

Handled this in the commit.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense! I strongly agree that any text-based resource should produce the same hashCode regardless of the formatting used. The 'hash' here is not an indicator of the actual text hash, but a hash for its 'semantic'. It should not change if something meaningless changed (e.g., if the format of line-endings changed).
However, I wonder how projects would end up with different XML files in their project? Is there some generation happening that produces different XML files (e.g., uses different line-endings on different platforms)?

@pjBooms pjBooms force-pushed the pjBooms/embed-compose-hot-reload-plugin branch from 2f47039 to 1bd3f05 Compare October 20, 2025 14:14
@pjBooms pjBooms force-pushed the pjBooms/embed-compose-hot-reload-plugin branch from 597fdf8 to b097888 Compare October 21, 2025 12:03
return readText().replace("\r\n", "\n").toByteArray().contentHashCode()
} else {
// Once a new text resource file is introduced, we have to catch it and handle its line endings.
check(type == ResourceType.DRAWABLE || type == ResourceType.FONT) { "Cannot calculate content hash for $type resource type!" }
Copy link
Member

Choose a reason for hiding this comment

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

why do you check font type?
why should binary PNGs throw the error?

Copy link
Member

Choose a reason for hiding this comment

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

add all type of resources in the test case, please. to check stability of the hashes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently we generate accessors only for font and drawable files (strings and raw files are handled separately). Here I am just checking this fact, so if we introduce a new text resource type, we could catch it and recall that we may need to do something with its line endings. It is reflected in the comment.

Copy link
Collaborator Author

@pjBooms pjBooms Oct 21, 2025

Choose a reason for hiding this comment

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

add all type of resources in the test case, please.

Do you mean add .svg and raster images?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added two more files

Copy link
Member

Choose a reason for hiding this comment

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

I don't agree. If we add a new text based resource type with a new extension, it will fail tests because hashes will be different. We don't need resource type here. And we don't need to check type == ResourceType.DRAWABLE as well. for any file based xml/svg resource we must do conversion on win machines

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, done.

(let's hope that we don't forget to add tests for new text files)

Copy link
Member

Choose a reason for hiding this comment

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

But just to double-check:
Any project would just check in those text files using a common line-ending, right? Will the git checkout then provide different line endings on different platforms?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct

@pjBooms pjBooms merged commit eb852d8 into master Oct 22, 2025
23 checks passed
@pjBooms pjBooms deleted the pjBooms/embed-compose-hot-reload-plugin branch October 22, 2025 09:11
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.

5 participants