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

[Modern] relay-compiler does not support complex folder-structures #1670

Closed
olve opened this issue Apr 21, 2017 · 7 comments
Closed

[Modern] relay-compiler does not support complex folder-structures #1670

olve opened this issue Apr 21, 2017 · 7 comments

Comments

@olve
Copy link

olve commented Apr 21, 2017

relay-compiler assumes a single directory components/ where modulename == filename.
This enforces a de-facto layout standard for all Relay Modern projects, which may or may not suit developers.

what relay-compiler assumes:
./src/components
    Admin.js
    AdminChildComponent.js
example complex src folder:
./src
    /components
        /Admin
            index.js
            styles.css

            /ChildComponent
                index.js
                styles.css
example case & error for complex folders:
someuser ~/D/g/some-project> relay-compiler --src ./src --schema ./schema.graphql

Error: FindGraphQLTags: Container fragment names must be `<ModuleName>_<propName>`.
Got `users`, expected `index_users`.
@mjmahone
Copy link
Contributor

This is not a folder complexity issue, but rather an issue with how you're naming your props on your component versus the fragment name. While there may be an issue with nested directories, it wouldn't manifest as this error.

If you have a container:

createFragmentContainer(MyContainer, {
  MyContainer_index_users: graphql`fragment MyContainer_users on Users { ... }`
})

it will throw. This is for a few reasons: in Compat mode, the fragment name is how we automatically create the prop key, and if you decide not to use an inner keyed object in your createFragmentContainer call, it's how we determine what the property name is. (Note, I can't fully remember the error, so it's possible I reversed which piece is in the fragment vs. the props).

@coleturner
Copy link
Contributor

While on the subject, how are nested modules and components (where it may or may not be index.js) supposed to work?

@olve
Copy link
Author

olve commented Apr 27, 2017

@mjmahone the exception is pertinent to the issue because it illustrates how Relay assumes module-names by filename (index.js -> index_fragment).

Another symptom of the problem is how your project directories will be cluttered by __generated__ folders unless you use a single components/ or containers/ folder

@josephsavona
Copy link
Contributor

The wording of the error is a bit confusing, we should address that. The compiler requires that fragments be named <FIleNameMinusExtension>_<propName> (internally we follow a convention of naming files the same as the module name, hence the wording of the error message).

I can see how this naming scheme could cause issues for applications that are using commonjs and particularly having lots of files named index.js, we'll have to consider how to handle this.

cc @kassens @leebyron @mjmahone

@gusbicalho
Copy link

gusbicalho commented Apr 27, 2017

From the documentation of FragmentContainer

A naming convention of _ for fragments is advised. This restriction is required while migrating from classic to modern APIs to allow for cross-compatibility.

I thought the naming format would be optional once we moved to Modern.

@mwalkerwells
Copy link

Looking at the source...


...you can pass a graphql fragment directly (without an object):

// App.js

export default createFragmentContainer(props => <Foo {...props} />,
  graphql`
    fragment App on User {
      ...Foo
    }
  `
)

This requires the use of the data prop: https://facebook.github.io/relay/docs/fragment-container.html#calling-component-instance-methods

@leebyron
Copy link
Contributor

A fix was added for "index" files for this issue, otherwise take a look at #2093

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

No branches or pull requests

7 participants