navi icon indicating copy to clipboard operation
navi copied to clipboard

navi-scripts build just stopped working

Open alexpricedev opened this issue 6 years ago • 10 comments

I'm not sure what I changed but for some reason now I'm getting

Couldn't find window.NaviScripts - did you call register()

I've read through the docs and double checked everything and I'm still unsure what is giving me this error... Is there anything obvious that I might be missing before I try and replicate it in a fresh repo?

alexpricedev avatar Apr 11 '19 21:04 alexpricedev

Hmm... there's nothing obvious that I can think of. As long as you're calling register() in index.js, and you're still using vanilla create-react-app, it should work.

jamesknelson avatar Apr 12 '19 07:04 jamesknelson

Only difference is that I'm running the TS flavour of CRA so i have an index.ts file. But I'm 99% sure I had it building fine with that setup. I'll do some digging. Thanks for your response!

alexpricedev avatar Apr 12 '19 12:04 alexpricedev

I've just taken another look into this and I got it working again by returning the result of the factory call in createScriptRunner.js rather than factory function itself:

  let test = await factory();
  if (!test) {
    throw new Error(
      `Could not find the "window.${
        config.appGlobal
      }" value in your entry point "${config.entry}".`,
    );
  }

  return test;
  // return factory;

I also had to amend the build func:

  let app = await createScriptRunner(config);

  for (let path of paths) {
    let dependencies = {
      scripts: new Set(),
      stylesheets: new Set(),
    };
    // let app = await scriptRunner({
    //     onScriptLoaded: (pathname) => dependencies.scripts.add(pathname),
    //     onStyleSheetLoaded: (pathname) => dependencies.stylesheets.add(pathname),
    // })

I assume those config options being passed to scriptRunner in build.js are essential, and this is not a complete workaround. But it is interesting to me that the test result has the routes key, and call in a different scope does not.

Any thoughts or feedback would be great!

alexpricedev avatar Apr 18 '19 13:04 alexpricedev

Thanks for getting back to me on this – it really shouldn't be necessary to do it that way.

Part of me wonders if maybe you're using mismatching versions of navi-scripts and navi. Could you let me know what versions are in node_modules?

Also, iirc the onScriptLoaded config option isn't used right now, but onStyleSheetLoaded is used to add stylesheet tags to the generated HTML – and you'll find styles don't load correctly in production without it (unless you're using styled-components, in which case you'll be fine).

jamesknelson avatar Apr 19 '19 01:04 jamesknelson

Okay good to know. Yeah the styles aren't working, but thats okay. I'm not in desperate need for a fix but it would be nice to get this wrapped up soon :)

Yesterday I upgraded navi so that's @ 0.12.6 and navi-scripts is @ 0.12.4.

alexpricedev avatar Apr 19 '19 08:04 alexpricedev

I encountered this issue with node v12.3.1, changed to v10.16.0 and it works.

pomppa avatar Jun 09 '19 13:06 pomppa

why is this issue closed, I still find that this happens in 0.13.6 :( even after making sure the releases match for react-navi and navi-scripts

decebal avatar Aug 27 '19 22:08 decebal

Okay, I’ve reopened this.

Could someone who is having this issue create a reproduction repo for me to look to?

Long term, I think the best solution is going to be to move away from loading the CRA output with jsdom. It’s a huge hack and maintenance is going to be a PITA. Instead, it should be possible to use universal-react-scripts as a drop-in replacement for react-scripts that creates a node bundle in addition to the web bundle. That should remove a lot of the hacks from navi-scripts, and help to avoid these kind of bugs in the future.

jamesknelson avatar Aug 28 '19 02:08 jamesknelson

hi @jamesknelson , I tried all the examples(create-react-blog, navi/examples) on my machine and it's throwing the error above. I also tried to use different versions (in sync) of navi-scripts, navi, react-navi (from 0.13.6 -> 0.12.8)

npm version: 6.11.2 node version: v12.8.0 os: macOS Mohave 10.14.5

I am willing to look into this as well, I tried to debug it but it doesn't help that I couldn't make it work first :(

decebal avatar Aug 28 '19 15:08 decebal

hello! @jamesknelson

I edited createScriptRunner.js to run Navi project with Node.js v16.

https://github.com/HAKASHUN/navi/commit/56e073c69a89732ff7ff69508d7f49c04b76ebb5

This works fine in my environment.

npm version:8.1.2 node version: v16.13.2 os: macOS Monterey 12.6

HAKASHUN avatar Nov 08 '22 09:11 HAKASHUN