Redwood Diagnostic Improvement: check Cell exports and warn if missing

Performing yarn rw check did not show warning for missing Query statement in a cell file. It's a bit difficult to pinpoint the issue if no warning is shown.

The screenshot below showed that no warning is showed for the missing Query when performing yarn rw check

Recently, I and @adriatic had made this mistake and it took some time to find out the root cause.

We received Invariant Violation error message which isn't obvious to me and several other community members to relate this error to the missing Query in a cell file.

unknown

cc @thedavidprice @Tobbe

cc @adriatic This appears to be the issue you are encountering in your project here: https://github.com/congral/redwoodblog/blob/master/web/src/components/BlogPostCell/BlogPostCell.js

There is no export Query

@aldonline using the current Model for a Cell, would you be able to provide some direction as to how someone could add this check to the current Diagnostics?

I'm not saying the current behavior is right. Or wrong. But it is a conscious decision by someone at some point in time.

From https://redwoodjs.com/docs/cells#how-does-redwood-know-a-cell-is-a-cell

Redwood looks for all files ending in "Cell" (so if you want your component to be a Cell, its filename does have to end in "Cell"), but if the file 1) doesn't export a const named QUERY and 2) has a default export, then it'll be skipped.

When would you want to do this? If you just want a file to end in "Cell" for some reason. Otherwise, don't worry about it!

So not exporting QUERY is a way to let someone have a file end with *Cell.js, without Redwood treating it as a cell. An escape hatch if you will.

Here's code from the structure package that explicitly handles this case packages/structure/src/model/RWCell.ts

/**
 * A "Cell" is a component that ends in `Cell.{js, jsx, tsx}`, has no
 * default export AND exports `QUERY`
 **/
@lazy() get isCell() {
  return !this.hasDefaultExport && this.exportedSymbols.has('QUERY')
}

And here's the same special treatment in our babel plugin that puts the cells in our withCellHOC packages/core/src/babelPlugins/babel-plugin-redwood-cell.ts

exit(path) {
  // Validate that this file has exports which are "cell-like":
  // If the user is not exporting `QUERY` and has a default export then
  // it's likely not a cell.
  if (hasDefaultExport && !exportNames.includes('QUERY')) {
    return
  }

Here's an old issue on the subject, with relevant input from @mojombo https://github.com/redwoodjs/redwood/pull/554#issuecomment-630340492

Here's also a previous discussion on Discord about what warnings we should have, and what should make a cell a cell with special rw sauce. https://discord.com/channels/679514959968993311/747258086569541703/800789585046405190

The TL;DR for the Discord discussion is: It would be nice to get a warning if RW finds a cell without a Success export

Missing the export statement should be diagnosed by lint - although it is too dangerous to extend lint for RWJS sake

Sent from my iPhone

On Mar 30, 2021, at 11:42 PM, Andrew Lam @.***> wrote:

Performing yarn rw check did not show warning for missing Query statement in a cell file. It's a bit difficult to pinpoint the issue if no warning is shown.

The screenshot below showed that no warning is showed for the missing Query when performing yarn rw check

Recently, I and @adriatic had made this mistake and it took some time to find out the root cause.

We received Invariant Violation error message which isn't obvious to me and several other community members to relate this error to the missing Query in a cell file.

cc @thedavidprice @Tobbe

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

Back from my trip, picking up from this point.

Explanation 1

@thedavidprice - it is true that my file BlogPostsCell.js does not contain the code

export const QUERY = gql`
  query BlogPostQuery($id: Int!) {
    post(id: $id) {
      id
      title
      body
      createdAt
    }
  }
`

The reason for this omission is simple: after going through the Tutorial for the umptieth tome (each new such trip was made in order to verify the most recent edition of the Tutorial), I fell in the same trap as before - copying the code from this "screenshot"

I forgot that the export const Query ... is intentionally missing there (as Rob explained to me he believed that keeping the complete code would just confuse the user as it would dilute his/her focus). As you see the omission had a different effect on me.

Explanation 2

I just added the missing snippet of the code to (BlogPostCell.js) - removed (locally) the node_modules folder and yarn.lock file

After rebuilding the app again (yarn install), I finally, run the yarn rw dev - and got the same crash as the very first time:

Uncaught Invariant Violation: Argument of undefined passed to parser was not a valid GraphQL DocumentNode. You may need to use 'graphql-tag' or another method to convert your operation into a document
    at new InvariantError (http://localhost:8910/static/js/app.chunk.js:5824:28)
    at invariant (http://localhost:8910/static/js/app.chunk.js:5836:15)
    at parser (http://localhost:8910/static/js/app.chunk.js:7617:88)
    at QueryData.push.../node_modules/@apollo/client/react/data/OperationData.js.OperationData.verifyDocumentType (http://localhost:8910/static/js/app.chunk.js:6709:88)
    at QueryData.push.../node_modules/@apollo/client/react/data/QueryData.js.QueryData.prepareObservableQueryOptions (http://localhost:8910/static/js/app.chunk.js:6915:14)
    at QueryData.push.../node_modules/@apollo/client/react/data/QueryData.js.QueryData.initializeObservableQuery (http://localhost:8910/static/js/app.chunk.js:6929:47)
    at QueryData.push.../node_modules/@apollo/client/react/data/QueryData.js.QueryData.updateObservableQuery (http://localhost:8910/static/js/app.chunk.js:6939:18)
    at QueryData.push.../node_modules/@apollo/client/react/data/QueryData.js.QueryData.execute (http://localhost:8910/static/js/app.chunk.js:6832:14)
    at http://localhost:8910/static/js/app.chunk.js:7482:151
    at useDeepMemo (http://localhost:8910/static/js/app.chunk.js:7519:42)
    at useBaseQuery (http://localhost:8910/static/js/app.chunk.js:7482:85)
    at Object.useQuery (http://localhost:8910/static/js/app.chunk.js:7354:87)
    at useQuery (http://localhost:8910/static/js/app.chunk.js:13929:57)
    at Query (http://localhost:8910/static/js/app.chunk.js:13973:53)
    at renderWithHooks (http://localhost:8910/static/js/app.chunk.js:62949:18)
    at mountIndeterminateComponent (http://localhost:8910/static/js/app.chunk.js:65775:13)
    at beginWork (http://localhost:8910/static/js/app.chunk.js:67013:16)
    at HTMLUnknownElement.callCallback (http://localhost:8910/static/js/app.chunk.js:51909:14)
    at Object.invokeGuardedCallbackDev (http://localhost:8910/static/js/app.chunk.js:51958:16)
    at invokeGuardedCallback (http://localhost:8910/static/js/app.chunk.js:52020:31)
    at beginWork$1 (http://localhost:8910/static/js/app.chunk.js:71923:7)
    at performUnitOfWork (http://localhost:8910/static/js/app.chunk.js:70735:12)
    at workLoopSync (http://localhost:8910/static/js/app.chunk.js:70666:5)
    at renderRootSync (http://localhost:8910/static/js/app.chunk.js:70629:7)
    at performSyncWorkOnRoot (http://localhost:8910/static/js/app.chunk.js:70252:18)
    at http://localhost:8910/static/js/app.chunk.js:59291:26
    at unstable_runWithPriority (http://localhost:8910/static/js/app.chunk.js:78237:12)
    at runWithPriority$1 (http://localhost:8910/static/js/app.chunk.js:59240:10)
    at flushSyncCallbackQueueImpl (http://localhost:8910/static/js/app.chunk.js:59286:9)
    at flushSyncCallbackQueue (http://localhost:8910/static/js/app.chunk.js:59273:3)
    at scheduleUpdateOnFiber (http://localhost:8910/static/js/app.chunk.js:69852:9)
    at Object.enqueueSetState (http://localhost:8910/static/js/app.chunk.js:60431:5)
    at PageLoader.push.../node_modules/react/cjs/react.development.js.Component.setState (http://localhost:8910/static/js/app.chunk.js:75363:16)
    at PageLoader.startPageLoadTransition (http://localhost:8910/static/js/app.chunk.js:12449:12)
index.js:1 The above error occurred in the <Query> component:

    at Query (http://localhost:8910/static/js/app.chunk.js:13969:3)
    at http://localhost:8910/static/js/app.chunk.js:14050:7
    at HomePage
    at PageLoader (http://localhost:8910/static/js/app.chunk.js:12397:5)
    at InternalRoute (http://localhost:8910/static/js/app.chunk.js:12912:3)
    at Route
    at main
    at BlogLayout (http://localhost:8910/static/js/app.chunk.js:92708:3)
    at Set (http://localhost:8910/static/js/app.chunk.js:11705:3)
    at RouteScanner (http://localhost:8910/static/js/app.chunk.js:13060:3)
    at ParamsProvider (http://localhost:8910/static/js/app.chunk.js:12583:3)
    at LocationProvider (http://localhost:8910/static/js/app.chunk.js:12284:5)
    at RouterContextProvider (http://localhost:8910/static/js/app.chunk.js:12799:12)
    at Router (http://localhost:8910/static/js/app.chunk.js:13021:3)
    at Routes
    at GraphQLHooksProvider (http://localhost:8910/static/js/app.chunk.js:13914:3)
    at ApolloProvider (http://localhost:8910/static/js/app.chunk.js:6482:21)
    at ApolloProviderWithFetchConfig (http://localhost:8910/static/js/app.chunk.js:13636:3)
    at FetchConfigProvider (http://localhost:8910/static/js/app.chunk.js:13825:3)
    at RedwoodApolloProvider (http://localhost:8910/static/js/app.chunk.js:13692:3)
    at FatalErrorBoundary (http://localhost:8910/static/js/app.chunk.js:13758:5)
    at App

React will try to recreate this component tree from scratch using the error boundary you provided, FatalErrorBoundary.

This means that the finding that the problem is in missing export Query ... - is indeed a red herring. It seems that my plan to make @forresthayes the torch carrier after I leave home was not all that successful, so let's please revive it again.

Summary and a fresh start

My comments above warrant a fresh staff:

The title of this issue: Redwood Diagnostic Improvement: check Cell exports and warn if missing has no bearing on my original plea for help, issued to find the reason for the error:

Uncaught Invariant Violation: Argument of undefined passed to parser was not a valid GraphQL DocumentNode. You may need to use 'graphql-tag' or another method to convert your operation into a document

The application contains my typed in code from the Tutorial, so there is either a problem in my typing (very likely) or a problem in the tutorial as it existed at the time of my typing.

Both @forresthayes and @andrew-hwahin responded with the confirmation that they reproduced my error, with the following caveats:

@forresthayes cloned the application that contains my typed in code from the Tutorial
@andrew-hwahin confirmed getting the same error - running his own app, not mine, like @forresthayes

Subsequently @andrew-hwahin wrote that he found the problem, running his own app - and he explained that the problem was in that missing Export Query ... statement.

Here is where the monkey jumps in the water:

@andrew-hwahin' discovery indicates the real serious problem (RWJS missing the Export Query ... statement.
My own code also missing the Export Query ... statement - but that is not the reason for my app getting Uncaught Invariant Violation: ... . When I added the missing snippet, my app behaved the same (issued the same error).

I am not addressing the reasons for @andrew-hwahin problem nor his solution, considering it is a separate issue.

My guess (that I will try to validate) is that the database request for the post by the giving post ID does not supply the ID (it remained undefined).

Part 1: Resolving the `Uncaught Invariant Violation`

Reproducing the error

Using your linked repo, I reproduce the error in this video:

https://s.tape.sh/rw7wIeLf

Resolving the error

I resolve the error in this video by adding the export const QUERY = gql ...` (from your comment above here)

https://s.tape.sh/rw7wIeLf

Note: because of my quick replication, I did not correctly set up the DB connection (I'm using GitHub Codespaces). This is the unrelated source of the SSL connection error.

@adriatic is there something you did differently in this Reproduce --> Resolve flow? And/or does my resolution also work for you? Please let me know.

Step 2: Improvements to the Tutorial

Recognizing that 1) ensuring the code and functionality are working is not the same as 2) providing the best Tutorial experience and instructions, what do you think about this approach:

You and I create a shared Google Doc (or another collaborative document editor) for collecting possible issues, hangups, out-of-date copy, and improvements... etc.

My objective would be to make "bugs" distinct from "needed Tutorial improvements" as well as to better track each along with status — I am definitely losing track of which is which, and which are resolved. Also, because the Tutorial state will continue to be in high flux for at least the next couple of months, I don't think it makes sense to resolve each Tutorial-related improvement incrementally. We simply don't have the bandwidth or focus across the community. However, if we could make it a clearly outlined project to be completed between the v1-release-candidates and the final v1.0.0 release, that would be amazing!

Lastly, I think your process for working through this is good. Specifically:

Discord: I ran into an issue. Here's what I did and how to reproduce. Does anyone see if I made a mistake to correct? And/or has anyone else experienced this as well?
Bug likelihood "confirmed" --> open GitHub Issue
[My suggestion above] Possible tutorial issue/improvement discovered --> add to Collaborative Google Doc

Thoughts, suggestions, reactions? Let's continue the discussion about this. But maybe over email (or other thread) to keep this GitHub Issue focused.

Thanks again!

@thedavidprice writes:

Lastly, I think your process for working through this is good. Specifically:

Discord: I ran into an issue. Here's what I did and how to reproduce. Does anyone see if I made a mistake to correct? And/or has anyone else experienced this as well? Bug likelihood "confirmed" --> open GitHub Issue [My suggestion above] Possible tutorial issue/improvement discovered --> add to Collaborative Google Doc Thoughts, suggestions, reactions? Let's continue the discussion about this. But maybe over email (or other thread) to keep this GitHub Issue focused.

I am very happy with your summary (above). Let's do that and let my participation be narrowed to execute this process with minimal load on your schedule. I will maintain my passionate focus on ensuring that the online documentation is precise, explanatory, and accurate - and will maintain and deviations in that space created for this purpose. I reviewed Google Doc and believe that a GitHub repo is significantly better for several reasons:

using markdown beats any other textual communications (after all RW us based on jaMstack).
we can maintain the actual code being used in discussions
the wiki can be used to easily include other people that we might need for the final resolution.

I just created a repository https://github.com/congral/rwdocs, and invited you there, proposing to use it as our playground. I for any reason this might not be kosher - please do the reverse

I really do like reading and writing (as well as improving) technical documentation, I am aware that the RWJS team did a great job already, so I do not mind relegating myself to the common good of "getting out" with most educational technical documentation ever.

I reviewed Google Doc and believe that a GitHub repo is significantly better for several reasons:

Ah, yes, you'd mentioned this previously. 💯 agree. I've accepted the invite and am following all change notifications on the repo! Thank you.

I really do like reading and writing (as well as improving) technical documentation

^^ I know it and I want to continue to harness your effort + energy! Not taken for granted. Just figuring out the best way to direct/harness in sync with a very dynamic schedule. 🚀

But watch out. Sooner or later I'm going to show you how we use Cypress to automate testing the Tutorial Steps. And then I'll just put you in charge of adding and fixing all the missing bits and pieces 😆

(FYI: there's a current Cypress issue with Windows that I don't fully understand. This is an obvious gap in our CI right now, meaning we only automate Tutorial testing for Linux (Ubuntu).

But watch out. Sooner or later I'm going to show you how we use Cypress to automate testing the Tutorial Steps. And then I'll just put you in charge of adding and fixing all the missing bits and pieces 😆

A great move, David. While I shared many dreams with you (most in the context of making VSCode a lot more involved tool for RWJS development), if I was you, I would task me with precisely what you just did. It is one of the most critical components of a successful startup launch - and I owe this task to the world (my single biggest mistake while I was in charge of the Nuasis (world's first cloud-based call center) was my neglect to testing. I hired an old friend of mine and discovered way too late that he had no idea what was he doing).

redwoodjs / redwood