redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
17.33k stars 994 forks source link

v0.26.x warning in browser console with yarn rw dev #1887

Closed adriatic closed 3 years ago

adriatic commented 3 years ago

Context: I am going through the Tutorial and started a bit differently than before - after running the Our First Page section, I run

yarn redwood setup custom-web-index

out of curiosity.

The next time I run yarn rw dev in VSCode terminal, I got this error

 Cannot find module '~__REDWOOD__USER_ROUTES_FOR_MOCK' from 'node_modules/@redwoodjs/testing/dist/MockProviders.js'

    Require stack:
      node_modules/@redwoodjs/testing/dist/MockProviders.js
      node_modules/@redwoodjs/testing/dist/customRender.js
      node_modules/@redwoodjs/testing/dist/index.js
      web/src/pages/HomePage/HomePage.test.js

      at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:306:11)
      at Object.<anonymous> (node_modules/@redwoodjs/testing/dist/MockProviders.js:32:5)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        1.478 s
Ran all test suites.

Note: I did not run anything related to jest, just the plain old yarn rw dev

The chrome / Brave browser shows

index.js:1 Warning: Did not expect server HTML to contain the text node " " in 
console.<computed> @ index.js:1

This might be the consequence of executing

yarn redwood setup custom-web-index 

command.


yarn run v1.22.5 $ c:\work\rw-samples\redwoodblog\node_modules.bin\rw info

System: OS: Windows 10 10.0.19041 Binaries: Node: 12.19.0 - ~\AppData\Local\Temp\yarn--1614468838033-0.07442912851026451\node.CMD Yarn: 1.22.5 - ~\AppData\Local\Temp\yarn--1614468838033-0.07442912851026451\yarn.CMD Browsers: Chrome: 88.0.4324.190 Edge: Spartan (44.19041.423.0), Chromium (88.0.705.81) npmPackages: @redwoodjs/core: ^0.26.2 => 0.26.2

Done in 4.20s.

adriatic commented 3 years ago

I removed the custom-web-index -no effect on Did not expect server HTML issue:

image

Tobbe commented 3 years ago

I'm not seeing Cannot find module '~__REDWOOD__USER_ROUTES_FOR_MOCK', but I can confirm the text node warning.

image

adriatic commented 3 years ago

@Tobbe - I was not sufficiently precise in my original report: the message

Cannot find module '~__REDWOOD__USER_ROUTES_FOR_MOCK' from 'node_modules/@redwoodjs/testing/dist/MockProviders.js'

appears in the VSCode PROBLEMS window as shown below:

image

Note that this problem shows twice - for HomePage.js and AboutPage.js each

peterp commented 3 years ago

The problem is the way that we're testing if <div id="redwood-app"> is empty: https://github.com/redwoodjs/redwood/blob/181b39131dc844b4e696d6108773ee317e1463cb/packages/web/src/entry/index.js#L10-L16

hasChildNode() is true if it contains whitespace.

I think we can continue by improving the way we determine if redwood-app is empty, but I think a better fix would be to explicitly insert a custom div into the html if we're pre-rendering.

We could replace these lines:

https://github.com/redwoodjs/redwood/blob/181b39131dc844b4e696d6108773ee317e1463cb/packages/prerender/src/runPrerender.tsx#L67-L71

To do something like:

// This is set by webpack by the html plugin
  const renderOutput = indexContent.replace(
    '<server-markup></server-markup>',
    `<div id="redwood-app-prerendered">${componentAsHtml}</div>`
  )

We could then search if that "id" exists:


const prerenderEl = document.getElementById('#redwood-app-prerendered')

if (prerenderEl) {
  ReactDOM.hydrate(<App />, prerenderEl)
} else {
  ReactDOM.render(<App />, document.getElementById('#redwood-app'))
}
dac09 commented 3 years ago

Thanks @peterp, yeah key things I've noted:

  1. Comments are considered childNodes as well, so just doing the above might not be sufficient
  2. We can only get HTML elements by using document.getElementById.children. Perhaps we can do the check like this?
const rootElement = document.getElementById('#redwood-app')

if (rootElement.children?.length > 0) {
  ReactDOM.hydrate(<App />, rootElement)
} else {
  ReactDOM.render(<App />,  rootElement)
}
peterp commented 3 years ago

Comments are considered childNodes as well, so just doing the above might not be sufficient

In my example above I was inserting a new div, people won't be able to add anything to it since it's created at pre-render time.

The reason why I'm suggesting we be explicit is because a user could still add things to index.html like:

<div id="redwood-app">
  <p>I'm loading...</p>
 <%= prerenderMarkup %>
</div>

And it would be an OK and valid thing to do.

adriatic commented 3 years ago

Thank you @peterp and @dac09 for sharing the details of how to address this issue.

thedavidprice commented 3 years ago

@dac09 are you planning to work on this one? And/or might you want to work on this (with Danny and Peter's guidance) via a new PR @adriatic?

adriatic commented 3 years ago

Happy to make a minimal app that demonstrates this issue, @thedavidprice . It is great timing for this as I am trying to write a document on debugging, which has a lot wider scope than fixing this issue 😄)

thedavidprice commented 3 years ago

Happy to make a minimal app that demonstrates this issue You're welcome to. At this point, however, I don't believe it's necessary for moving forward with a fix. I'll defer to @dac09

Docs: Windows recommendations and Debugging I have many things I’m catching up on this week, including the Windows wiki content and definitely the debugging document! Thank you for your patience with me as the past two releases had my head buried in GitHub.

adriatic commented 3 years ago

At this point, however, I don't believe it's necessary for moving forward with a fix

I already created it, so we can use it to verify the fix

adriatic commented 3 years ago

I finished rebuilding the tutorial blog app up to and including the layouts section. I plan to finish this tomorrow - but today I can state that the problem Cannot find module '~__REDWOOD__USER_ROUTES_FOR_MOCK' reported at the beginning of this thread is a red herring - the problem is with VSCode reaction on the first insertion of any new page into the project. This most often happens with "test" pages, but I saw it elsewhere as well.

@thedavidprice , @dac09 , @peterp

Please see more in the issue Remarks while building this tutorial app to the point, so you can agree with my assessment to deal with this outside of the context of the pretender issues.

dac09 commented 3 years ago

@dac09 are you planning to work on this one?

I'm happy to leave it to Nik :)

The reason why I'm suggesting we be explicit is because a user could still add things to index.html like:...

Hmm sure, a little unusual for me personally, but I can see the benefit.

@adriatic how do you feel about doing a PR to fix the warning? Please do ping me on the draft PR if you need any help getting it through.

adriatic commented 3 years ago

Executive summary for of the 1887 issue.

All parts of the 1887 issue can be reduced to two points:


1. The problem described above and copied here

image

is a real issue and it happens after the command yarn create redwood-app ./redwoodblog is executed. It was confirmed by @Tobbe and is not explained yet what is the cause of it


2. The Cannot find module '~__REDWOOD__USER_ROUTES_FOR_MOCK' is likely caused by VSCode being slightly confused with the generated skeleton code for *.test.js



Relevant details

This issue, which @dac09 renamed to "v0.26.x warning in browser console with yarn rw dev" is the first time mentioned in Test suite failed to run #1829 and has nothing to do with running tests.

  1. This issue is generated by VSCode as a response to a new code generated for a project already "opened" by VSCode. Example: yarn redwood generate page home /

  2. It shows up in the VSCode PROBLEMS view, as

    ● The test suite failed to run
    
    Cannot find module '~__REDWOOD__USER_ROUTES_FOR_MOCK' from 'node_modules/@redwoodjs/testing/dist/MockProviders.js'
    
    Require stack:
      node_modules/@redwoodjs/testing/dist/MockProviders.js
      node_modules/@redwoodjs/testing/dist/customRender.js
      node_modules/@redwoodjs/testing/dist/index.js
      web/src/layouts/BlogLayout/BlogLayout.test.js
    
      at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:306:11)
      at Object.<anonymous> (node_modules/@redwoodjs/testing/dist/MockProviders.js:32:5)
  3. In order to verify this behavior, run the following steps (in the VSCode terminal (console) view)

    yarn create redwood-app ./redwoodblog
    cd redwoodblog
    yarn redwood generate page home /
    yarn redwood generate page about

At this point you should see the error warnings mentioned above (Item 2) note that the problem appears semi-randomly: I saw the same problem (Item 2) after adding just the home page, and today it took adding About page (yarn redwood generate page about)

In all cases, VSCode seems unhappy with the generated code .test.js - and even that behavior depends on VSCode state of processing new .test.js. By this I meant this:

rw generator writes the 3 files .js, .test.js and .stories.js in the newly added folder . This event triggers VSCode to parse initially the newly added files - and it is that moment when the error text shown in item 2 above gets displayed. Note that VSCode treats these three files as new and requires the user to "save" these new files to be a part of the project. At that time these files are reparsed and the previously created error message (Item 2) vanish.

adriatic commented 3 years ago

Similarly to the Cannot find module '~__REDWOOD__USER_ROUTES_FOR_MOCK' there are other instances when VSCode issues warnings too soon (before reparsing the newly added code after the code is saved (Ctrl+S). Here is the example created at the moment when the generated (generic) code for BlogLayout.js is replaced with

// web/src/layouts/BlogLayout/BlogLayout.js

import { Link, routes } from '@redwoodjs/router'

const BlogLayout = ({ children }) => {
  return (
    <>
      <header>
        <h1>Redwood Blog</h1>
        <nav>
          <ul>
            <li>
              <Link to={routes.about()}>About</Link>
            </li>
          </ul>
        </nav>
      </header>
      <main>{children}</main>
    </>
  )
}

export default BlogLayout

when you get two errors:

image

Both these errors disappear when the user saves this just added code.

The second error is caused by the last line export default BlogLayout, which is obtained by click on the Copy button in the tutorial text, which does not have the proper EOL indicator.

Note: this article discusses the VSCode behavior on the "save" command and recommends the following setting

  "editor.codeActionsOnSave": [
    "source.organizeImports",
    "source.fixAll" 
  ]
adriatic commented 3 years ago

Finally, it dawned on me that all the hoopla about '~__REDWOOD__USER_ROUTES_FOR_MOCK' from 'node_modules/@redwoodjs/testing/dist/MockProviders.js' stems from the fact that VSCode tries to run all the *.test.js files without being specifically asked to do that:

image

Are the generated test files in their initial form ready to be run>

jvanbaarsen commented 3 years ago

@adriatic Yeah the the files should generate a green test suite. Do you happen to know what command VSCode is using to run the tests? Because if you run it outside of the $ rw test context you're missing a couple of important ENV vars. So if it runs something like yarn test, then that is the reason you're getting these errors.

adriatic commented 3 years ago

@jvanbaarsen this is reducing to be the case of bad VSCode settings, which result in VSCode running the tests right after it discovered the presence of the tests files. This should not be the case in the development mode.

jvanbaarsen commented 3 years ago

For what it's worth, I'm also seeing the following warning on MacOS (0.26.x)

Warning: Did not expect server HTML to contain the text node "
      " in <div>.
adriatic commented 3 years ago

@thedavidprice @dac09 allow me to keep this issue open because the fix addresses only the problem diagnosed diagnosed by @peterp above. My original issue

Cannot find module '~__REDWOOD__USER_ROUTES_FOR_MOCK' from 'node_modules/@redwoodjs/testing/dist/MockProviders.js'

is caused by "inadequate" VSCode settings, which do not correctly address the application testing (the testing starts before it should). I will verify this and report the solution as a separate issue.

thedavidprice commented 3 years ago

is caused by "inadequate" VSCode settings, which do not correctly address the application testing (the testing starts before it should). I will verify this and report the solution as a separate issue.

Ah, thank you @adriatic The PR we merged including a fix for the "Did not expect server HTML" issue will be included in this week's coming v0.28. If you every want to test the current unstable releases, you can run yarn rw upgrade --tag canary to install the most recent canary packages.

If you do find the Cannot find module '~__REDWOOD__USER_ROUTES_FOR_MOCK' persists (as I'm assuming it does), it would be very helpful to organize via a new issue. You can always add a link back to this issue as a reference.

Thanks!

adriatic commented 3 years ago

Yes, I am immediately switching to testing section (have not touched that yet) and will use if in the context of the Tutorial where I encountered that Cannot find module '~__REDWOOD__USER_ROUTES_FOR_MOCK' issue.