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

Workaround for Name Too Long violations with responseBased code generation #5480

Closed

Conversation

AdamMTGreenberg
Copy link
Contributor

Preface

For starters, I deeply apologies since this PR may violate:

Surprise us with big pull requests. Instead, file an issue and start a discussion so we can agree on a direction before you invest a large amount of time.

It was a need we had for our current implementation within our consumption of the Apollo Kotlin code. I had to build a functional system for resolving this error in our current use, so regardless of the formal code review process, I had to come up with a working system.

Intro

This PR servers to help resolve the one of the known limitations of working with responseBased model generation.

This solution allows us to take the output of the error output during code compilation where the class name is output in the form of Class$With$A$Really$Long$Name$That$Breaks$Your$Build and dictate how to flatten it via a secondary input in the form of Class$With$A$Really$Long$Name to flatten the Name$That$Breaks$Your$Build inner class.

We dictate this via a gradle flag named flattenExplicitly which allows the user to dictate a JSON file which contains the set of classes which should be explicitly flattened in this manner. Multiple levels of the same class can be broken up at multiple levels in a successful manner. The JSON has the form of:

[
  {
    "className": "Class$With$A$Really$Long$Name$That$Breaks$Your$Build",
    "extract": "Class$With$A$Really$Long$Name",
    "renameTo": ""
  }
]

(note: at the current time I did not implement the renameTo functionality since I wanted to make this PR as small as possible while I received review/we discussed)

The bulk of the logic is in the Flatten.kt file where we build a tree of the potential explicitly flattened files, create dequeues of the linked IrModels and IrModelGroups to flatten, and then finally rebuild the top-level IrModelGroup. The majority of the additional code is passing the vale of flattenExplicitly throughout the system and supporting test classes and fake GraphQL genreated output and data. Additionally there are a few data models to try and simplify the hierarchy of the explicitly flattened generated classes.

Notes

  • This system, similar to flatten, does not support java code gen
  • This system, intended to resolve responseBased limitations, does not support operationBased models (in tests it works - but there are issues with adapters which have yet to be resolved)
    • As such, the operationbased test cases can be omitted and I was about to delete them from the review, but left them in case anyone wanted to review the shape of their output.
  • I have back ported it to Apollo 3.x but wanted to open this PR first to ensure that I am not inundating the code reviewers with a double whammy of a PR to review

… gradle for making the system ingest a input.json file, which it can then take and create a tree definition of the system as a whole which will allow us to perform a fix for the Name Too Long exceptions in the Apollo system
…ll flatten out the modules which we want when the name is too long. System will first check to see if the existing operation or fragment matches the naming convention and then split the IrModelGroups up, and then finally ensure that there are no name collisions.
…de setup, we can omit them in the test cases
Copy link

netlify bot commented Dec 14, 2023

👷 Deploy request for apollo-android-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 152e1bd

@martinbonnin
Copy link
Contributor

Hi 👋 thanks for looking into this!

responseBased codegen has its quitcks indeed, one of them being the NameTooLong issue on MacOS filesystems. The other main one being the exponential blow up in generated code. I think it's less of an issue in typescript because the types are erased at runtime so the workload is a bit lighter for the compiler (although it still grows exponentially).

Because of that exponential blowup, we strongly recommend using operationBased, which behaves in a lot more predictable manner.

That being said, responseBased will stay, for experiments like this one, possibly testing improvements in the Kotlin language and for high-communication teams that are willing to accept the tradeoffs.

Back to your proposal, 2 early feedbacks:

  1. It'd be a perfect candidate for a compiler API: no need to tunnel the parameters in the Gradle plugin + no need to bake the implementation in the compiler, you can iterate in your codebase much faster. I'll try to do something for v4.
  2. I wonder how far @JvmName could get you. I think it would rename the name of the class file. If using a$a$a$a, a$a$a$b, etc... you could possibly keep using the long Kotlin names but bypass the filesystem limitations. Would that be an option?

@martinbonnin
Copy link
Contributor

Heads up there are now Apollo Compiler Plugins to customize the IR and flattening of models:

Would that work for your use case?

@martinbonnin
Copy link
Contributor

@AdamMTGreenberg I'm going to close this one as the codebase has largely changed and there are now Apollo Compiler Plugins to help with this in hopefully a more flexible way.

Let me know if you want to deep dive into this again, I'm happy to dive with 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