Apollo-tooling: Leave typenames the way they are defined in queries for Flow/Typescript targets

[chrishandorf]

I'm on the latest release as of this writing: [email protected]

I have a Product Bill Of Materials (BOM) structure named as follows:

GraphQL input

input InputProductBOMItem {
...
}

Generated interface

export interface InputProductBomItem { // The "om" should NOT be lower case
...
}

This causes errors everywhere InputProductBOMItem is referenced.

[lewisf]

Thanks for reporting! This is a duplicate of #72.

[chrishandorf]

@lewisf I thought it was a duplicate too, but #72 was fixed and released with #79 wasn't it? I'm running the latest release 0.10.11 and its still happening.

[chrishandorf]

@lewisf I think the difference here is that this string is neither camelCase nor PascalCase. Why doesn't the apollo-codegen just leave all type names as-is rather than trying to modify them?

[lewisf]

@chrishandorf I think it's because popular style guides have recommended that style for both flow/typescript which is why PascalCase'ing was added, but I agree that it leads to a lot of unpredictability with the type names.

I'd like to personally move it to just use names as is because it seems like this issue has come up multiple times and PascalCase'ing the names is starting to feel like unpredictable behavior. Maybe we should discuss rather or not that is better behavior in general for developer experience?

@martijnwalraven, @rricard if you have a chance to chime in, that would be awesome.

[lewisf]

I just changed this to an issue and we're going to consolidate all discussion around casing here, which includes: #72

[thekevinbrown]

We also have a case where we'd like to namespace with underscores since everything ends up in one file:

If I name the query SomeComponent_GetThing it gets generated as SomeComponentGetThingQuery which is harder to read, and I'd like it to leave the names I give alone if possible.

Of course even more amazing would be if it could generate the types alongside the query file rather than jamming them all into a single file so we don't have to namespace them in the first place and can just import the correct ones where they're needed. 👍

[rricard]

@lewisf just added it the 1.0 milestone, this is indeed a bug, we should not enforce conventions since it breaks the query naming.

@martijnwalraven do you see any issues with the swift target if we stop enforcing this? We can eventually keep it only for swift...

[martijnwalraven]

@rricard: As far as I know, the case conversions are part of the individual targets and not the compiler, so there shouldn't be any issue removing them from TypeScript/Flow.

[rricard]

Okay, so let's only target flow and TS on this issue. And you are right I did not see anything in the IR, I may have taken the decision to enforce it for no reason. This is just a simple function call to remove. If someone would like to take it as a PR, feel free to ping me or I can handle that...

For someone wanting to do this, here is where to look ;) :

• Flow: https://github.com/apollographql/apollo-codegen/blob/master/src/flow/codeGeneration.js#L87:L109, https://github.com/apollographql/apollo-codegen/blob/master/src/flow/codeGeneration.js#L170
• TS: https://github.com/apollographql/apollo-codegen/blob/master/src/typescript/codeGeneration.js#L100:L114, https://github.com/apollographql/apollo-codegen/blob/master/src/typescript/codeGeneration.js#L174

[rricard]

I'm going to do it since it has been one week without anyone claiming the issue and this needs to be fixed.

[leethree]

This change broke our current workflow because we use camelCase for fragment/query names but PascalCase for Flow type names. Forced-pascal-casing worked great for us. Is there any chance that you could make it into an option?

[rricard]

@leethree adding options is a good escape hatch to make everyone happy and makes sense when we're approaching a transform that considerably changes the output... However, since we're trying to stabilize this project, adding too much options keeps a great level of branching throughout the project. I would advise you to stay on the version before this one, migrate all non-enforced pascal case names or, use types in camelCase, and migrate back to this version.

[leethree]

@rricard OK. thanks for the advice!

[rricard]

@leethree I'm still trying to figure out how we could handle options more easily and gracefully so someone can do a PR to adress the problem you have if needed. All I want is have everything properly structured and documented.