Support custom module-name generator in relay-compiler

[martinandert]

I know this has been shot down in #2101 already, but not being able to provide our own module-name generator is a major obstacle for us to truly adopting Relay in our codebase. Here's why:

We have several hundred components, and their file names aren't unique, since we don't use Haste for module resolution, but rather namespace our modules by putting their files in (sub)directories. For example, given the following file paths for three different List components,

src/desktop/account/orders/list.tsx
src/mobile/account/orders/list/index.tsx
src/some/other/list.tsx

the compiler's built-in getModuleName function turns all these file paths into the same module name, "list". Needless to say, the compiler will complain about module names not being unique.

Of course, we could give our components unique names, e.g.

src/desktop/account/orders/list/DesktopAccountOrdersList.tsx
src/mobile/account/orders/list/MobileAccountOrdersList.tsx
src/some/other/list/SomeOtherList.tsx

in order to get the desired module names, but seeing a naming scheme like this hurts my eyes and feels like a very ugly hack just to please the compiler. And no, ditching the namespacing-via-directories approach isn't really an option for us.

Also, relaxing the constraint of unique file names (see #2093) doesn't really help, since I consider it a feature that the compiler bails out when it detects this. It also helps when logging the operation name in fetchRelay. And having all Relay artifacts in one folder is also nice.

Besides that, I think it's a problem that getModuleName isn't exposed publicly, because then tools like eslint-plugin-relay have to copy its implementation to their codebases, which has the potential to break if the original implemention is changed.

To overcome these issues, we currently have a scripts.postinstall hook in our package.json which replaces the implementations of the getModuleName function in "node_modules/relay-compiler/bin/relay-compiler" as well as "node_modules/eslint-plugin-relay/src/utils.js" with our own implementation, which turns the file paths from the first example above into unique module names such as

DesktopAccountOrdersList
MobileAccountOrdersList
SomeOtherList

respectively.

It would be nice if we wouldn't have to resort to an ugly hack like this just to outfox the compiler.

The aforementioned issues are addressed by this pull request, in that it

• introduces a new generateModuleNameFunction config option which allows users to pass their own module-name generator function, either as an actual function or as a path string which points to a local module that default-exports such a function (matching the behavior of the existing persistFunction config setting), and
• exposes a public getGenerateModuleNameFunction(relayConfig) export which tools like eslint-plugin-relay can utilize instead of having to copy-and-paste code from the Relay codebase.

Please let me know your thoughts.

Thanks for such a great library!

[martinandert]

It's been a week since I opened the PR. Please let me know if you need anything else from me.

[maraisr]

I'm by no means am an anybody - but I think this would be an amazing addition. 👏

I am in a similar boat that you, but we got around that by firstly satisfying Relay, and then we put anything after the first bit, to then make it unique.

ie:

// src/desktop/account/orders/list.tsx
ListDesktopAccountOrders_Query
// src/mobile/account/orders/list/index.tsx
ListMobileAccountOrders_Query
// src/some/other/list.tsx
ListOther_Query

as long as the derived module name is in the start, and ends with what it wants you to, you're good.

Simply; List%something else%, has worked quite well for us. And relay throws a woobly when the names to conflict.

But yeah in saying that, it would be amazing to have consistency in those names too - so you don't start going ListUniqueA, ListUniqueB, etc... than actually making them be sensible.

[pgeimer]

@maraisr Sadly this approach doesn't work for fragments. It has to be the exact name.

Parse error: Error: RelayFindGraphQLTags: Container fragment names must be 
`<ModuleName>_<propName>`. Got `ListCustomer_list`, expected `List_list`. in
"components/Customer/List.js"

[alloy]

I can’t really speak to the problem this solves, I can only say that at a glance the code changes look good to me 👍 /cc @jstejada

[eMerzh]

hey @martinandert , just stumble on this issue, i've got a bunch of indexes.js that i want to add with an @inline .... but i'm stuck with the Error: RelayFindGraphQLTags: Fragment names in graphql tags must be prefixed with the module name.

any way you could resolve conflicts/address the comments? so it has a chance to move forward?

[martinandert]

[...] any way you could resolve conflicts/address the comments?

@eMerzh I've rebased the PR against master in order to resolve the conflicts.

Which comments do you think need to be addressed?

[eMerzh]

@martinandert nothing specific :)

thanks for the rebase 👍

[jufemaiz]

👀

[martinandert]

Now that ~10 months have passed (and efforts are made to port the compiler to Rust), I'm wondering whether this was even considered / looked at by the maintainers. It would be great to get some feedback on it.

/cc @kassens

[josephsavona]

Our apologies for not following up on this sooner. As you can imagine we have limited resources, and we prioritize looking at critical bug fixes or improvements where we (core team) and the contributor have aligned on the change and the approach in advance.

This is something we can consider for the new Rust-based compiler. cc @tyao1 and @kassens

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[martinandert]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Adding a comment to prevent the PR from being autoclosed by the bot.

[hehex9]

any chance ?

[charklewis]

Are there any updates on this?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jufemaiz]

Ummm … team? Any movement or review?

[tony]

@martinandert Are you willing to update this?

in order to get the desired module names, but seeing a naming scheme like this hurts my eyes and feels like a very ugly hack just to please the compiler. And no, ditching the namespacing-via-directories approach isn't really an option for us.

I'm in the same boat. I just bumped into this and further surprised namespacing via directories isn't just default behavior (assuming there's no naming collision)

[zygopleural]

This would be super helpful, still. Any updates on this for either this compiler or the "new" Rust implementation?