GMOD / jbrowse-react-app-vanillajs-demo

0 stars 0 forks source link

Example no longer seems to work: "Nothing returned from render." #1

Open RMCrean opened 3 months ago

RMCrean commented 3 months ago

Hello,

Thanks very much for this example. When I click on the link in the README to the online example it did not show a Jbrowse instance. On inspecting the webpage I got:

Uncaught ReferenceError: JBrowseReactLinearGenomeView is not defined
    <anonymous> https://jbrowse.org/demos/app-vanillajs/genomeView.js:5

I then cloned the repo and ran it locally and got the following message:

Error: Minified React error #152;

which is:

Nothing was returned from render. This usually means a return statement is missing. Or, to render nothing, return null.

see: https://react.dev/errors/152?invariant=152&args%5B%5D=Highlight

Weirdly if I change the version of react and react-dom in the index.html to 16 or 18 the error goes away and everything seems to work fine. e.g.:

    <script
      src="https://unpkg.com/react@18/umd/react.production.min.js"
      crossorigin
    ></script>
    <script
      src="https://unpkg.com/react-dom@18/umd/react-dom.production.min.js"
      crossorigin
    ></script>

Fair warning that I'm very much learning JS and have no experience with React so I have no idea if that is an appropriate fix.

Also, I did see this about swapping from React 17 to 18 which makes me more confused about 17 not working but 18 seeming to work:

react-dom: ReactDOM.render has been deprecated. Using it will warn and run your app in React 17 mode.

From: https://react.dev/blog/2022/03/08/react-18-upgrade-guide#deprecations

Seen as how ReactDOM.render is used in "genomeView.js".

Thanks for reading this, I'm more than happy to help with any debugging or testing or provide more info.

cmdcolin commented 3 months ago

thanks for reporting this.

Uncaught ReferenceError: JBrowseReactLinearGenomeView is not defined

https://jbrowse.org/demos/app-vanillajs/genomeView.js:5

ya this was actually fixed in the repo but wasnt deployed to the web it looks like!

Error: Minified React error #152;

yes this was a double add on bug that was actually caused recently, where some new code (written just in the past months) was returning undefined to indicate "render nothing" instead of null (which is the proper way to render nothing). we have mostly done our testing on the latest react 18 now so we missed this, but we try to nominally support older versions like 17 so we can add a fix for older versions, but certainly i think it's good to run the latest so we can also update this example repo to use react 18

react-dom: ReactDOM.render has been deprecated. Using it will warn and run your app in React 17 mode.

i will update the code snippet in this repo, there is just a weird thing in the react 17->18 migration where we change "ReactDOM.render" to something called "createRoot"

random blog explaining the null vs undefined thing https://blog.saeloun.com/2022/04/14/react-18-allows-components-to-render-undfined/

cmdcolin commented 3 months ago

should be fixed somewhat here. still will cause errors on react 17 till we fix bug and do another release in main product, but use with react 18 should work https://jbrowse.org/demos/app-vanillajs/

note that there are some drawbacks to using this vanillajs usage currently, it notably doesnt use our webworker optimization, and has a larger bundle size than the vite/cra/nextjs usage examples. just a note :) possibly the web workers could be added, and even code splitting for the vanilla js (if we leaned really hard into esm modules) but it would be a bit of work

RMCrean commented 3 months ago

Hey, thanks so much for the quick reply and the detailed explanations about what was happening! Good to know I can update the code according to how you have now done it in the repo and use react 18.

note that there are some drawbacks to using this vanillajs usage currently, it notably doesnt use our webworker optimization, and has a larger bundle size than the vite/cra/nextjs usage examples. just a note :) possibly the web workers could be added, and even code splitting for the vanilla js (if we leaned really hard into esm modules) but it would be a bit of work

Thanks for the heads up, we are using Hugo so I'm not sure how easy it would be to use one of those options instead. Do you happen to have a recommendation for that if we are using Hugo for the rest of the site?

cmdcolin commented 3 months ago

you will probably be fine with the vanillajs build. if you are seeing some performance problems though e.g.

a) want to reduce bundle size/the feeling of the page you have is sorta heavy b) you have large bam/cram files being rendered

if neither situation applies to you, you can probably skip it. if either situation DOES apply to you, then you can try looking at e.g. hugo-webpack or hugo-vite packages, and cross reference our CRA which is webpack based or vite demos in the embedded demos :) https://jbrowse.org/jb2/docs/embedded_components/ also can consider just embedding the full jbrowse-web app (which automatically has code splitting and web workers) too e.g. in an iframe or something like this

RMCrean commented 3 months ago

Good to know and thanks so much for the tips! Your help is really appreciated.

Feel free to close this issue if you deem it appropriate and thanks again!