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

[WIP] Preview fix #1

Open
wants to merge 16 commits into
base: base-v2023.03.21-aswb-stable
Choose a base branch
from

Conversation

colinrgodsey
Copy link

@colinrgodsey colinrgodsey commented Mar 29, 2023

Fix XML previews, resource resolution, and compose previews.

Fixes: bazelbuild#4046, bazelbuild#2765

@colinrgodsey colinrgodsey changed the title Colin/previews fix [WIP] Preview fix Mar 29, 2023
@colinrgodsey colinrgodsey changed the base branch from master to base-v2023.03.07-aswb-stable March 29, 2023 19:28
@colinrgodsey colinrgodsey marked this pull request as draft March 29, 2023 19:28
@colinrgodsey colinrgodsey force-pushed the colin/previews_fix branch 3 times, most recently from 5179035 to 3094cf0 Compare March 30, 2023 17:20
@colinrgodsey colinrgodsey marked this pull request as ready for review March 30, 2023 17:22
@@ -268,4 +309,43 @@ private static VirtualFile getJarRootForLocalFile(VirtualFile file) {
public static boolean isEnabled() {
return enabled.getValue();
}

private class ModuleCache {
Copy link
Author

Choose a reason for hiding this comment

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

not sure if its worth trying to extend the existing SyncCache pattern

@@ -24,7 +24,7 @@
*/
public class ExperimentComposeStatusProvider implements ComposeStatusProvider {
private static final BoolExperiment composeEnabled =
new BoolExperiment("aswb.force.enable.compose", false);
new BoolExperiment("aswb.force.enable.compose", true);
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this? I wasn't actually sure. Someone in the Bazel Slack suggested it, but looking through the codebase, it didn't seem totally necessary. This seemed more of a "last resort".

@colinrgodsey colinrgodsey force-pushed the colin/previews_fix branch 2 times, most recently from af2f9d5 to 82e90ef Compare April 26, 2023 18:48
@colinrgodsey
Copy link
Author

Added a new commit that fixes source code resolution for dependencies (aars and jars)

@AlexBeggs
Copy link

We are interested in getting this working also. We did some testing, and I am not sure if you are aware but this doesn't render for a constraintlayout when using pinned resources. I created a sample repo to test against and some information related to it.

https://github.com/AlexBeggs/sample-constraintlayout-bazel-app

@colinrgodsey
Copy link
Author

@AlexBeggs ahhh yeah, I noticed that. not really sure where to start with debugging that 🤔 it apparently happens on gradle sometimes, so i wonder if there's some kind of classpath conflict, or possible if intellij is expecting this to be loaded from some specific library instead of the one used in the compiled project?

@AlexBeggs
Copy link

AlexBeggs commented Jun 2, 2023

This is where the is_source value is used https://github.com/bazelbuild/intellij/blob/d5d0ea12e48052555feb23b8d1913a93802f7f5c/aswb/src/com/google/idea/blaze/android/sync/importer/BlazeAndroidWorkspaceImporter.java#L238

If this check is disabled it renders, however I don't know what other side-effects this will cause by removing this check.

@colinrgodsey
Copy link
Author

ohh thats really interesting! I wonder why that matters....

@colinrgodsey
Copy link
Author

@colinrgodsey
Copy link
Author

ok wow, I think this definitely unlocked a lot of possibilities here. I think in general something is seriously wrong with the blocking of "generated resources".

@colinrgodsey
Copy link
Author

alright, so i just found the generated_android_resource_directories option for the intellij .bazelproject files. That's allowed referencing resources from our local modules, which was always broken for us (using an XML that referenced a resource from another module). So I think there's just an issue allowing this from AARs. I found another option that might help, going to test

@colinrgodsey colinrgodsey changed the base branch from base-v2023.03.07-aswb-stable to base-v2023.03.21-aswb-stable June 2, 2023 19:13
@colinrgodsey
Copy link
Author

colinrgodsey commented Jun 2, 2023

alright, i have a WIP commit on this branch now: 815c76d

from what I'm reading about, the blocking of generated resources is for performance reasons, I guess because there was redundant lookup methods? well, guessing one of those redundant methods doesnt work anymore, so unlocking generated resources entirely seems to just fix soooo much.

Copy link

github-actions bot commented Dec 4, 2023

Thank you for contributing to the IntelliJ repository! This pull request has been marked as stale since it has not had any activity in the last 6 months. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-review". Please reach out to the triage team (@bazelbuild/triage) if you think this PR is still relevant or you are interested in getting the PR merged.

@github-actions github-actions bot added the stale label Dec 4, 2023
Copy link

This pull request has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!

@github-actions github-actions bot closed this Dec 18, 2023
@pswaminathan pswaminathan reopened this Dec 20, 2023
@github-actions github-actions bot removed the stale label Dec 21, 2023
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.

4 participants