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

Improve error message of collocated fragments. #7

Merged
merged 3 commits into from
Aug 18, 2023

Conversation

MartinNuc
Copy link
Collaborator

@MartinNuc MartinNuc commented Aug 18, 2023

It took us some debugging to find out that the problem is that we need to have _ in the name of a fragment :-) Tried to improve the error message. If a better error message comes to your mind, I am all ears! :-)

'needs this information, that module should directly define a ' +
'fragment querying for that data, colocated next to where the ' +
'data is used.\n'
`this module does not use it directly or it the fragment is named incorrectly. If a different module ` +
Copy link
Member

Choose a reason for hiding this comment

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

I'd even remove the first part of the error message. Let's make it clear that the problem is a) name of the imported component and b) the underscore.

Copy link

@jaroslav-kubicek jaroslav-kubicek left a comment

Choose a reason for hiding this comment

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

great, thanks! Just pls fix some typos

`needs the data from this fragment, that module should directly define it's own fragment ` +
`to query for it's own data, and such fragment should be spread in the parent component.` +
`The naming convention should be <nameOfComponentCamelCase>_<optionalSuffix>. ` +
`The <nameOfComponentCamelCase> should match the import name. Optinal suffix should be separated ` +

Choose a reason for hiding this comment

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

Suggested change
`The <nameOfComponentCamelCase> should match the import name. Optinal suffix should be separated ` +
`The <nameOfComponentCamelCase> should match the import name. The optional suffix should be separated ` +

'needs this information, that module should directly define a ' +
'fragment querying for that data, colocated next to where the ' +
'data is used.\n'
`this module does not use it directly or it the fragment is named incorrectly. If a different module ` +

Choose a reason for hiding this comment

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

Suggested change
`this module does not use it directly or it the fragment is named incorrectly. If a different module ` +
`this module does not use it directly or the fragment is named incorrectly. If a different module ` +

@MartinNuc MartinNuc merged commit 68291db into main Aug 18, 2023
3 checks passed
@MartinNuc MartinNuc deleted the SKIP-123-collocated-fragments-message branch August 18, 2023 10:39
Comment on lines +162 to +163
`The <nameOfComponentCamelCase> must match the import name. The optional suffix should be separated ` +
`by underscore (usually when you need to pass multiple fragments to the same component).\n`
Copy link
Member

Choose a reason for hiding this comment

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

Why we didn't make it explicit? We have information about the names of fragments right? This could be better for DX 🙌

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