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

navi-scripts build just stopped working #83

Open
alexpricedev opened this issue Apr 11, 2019 · 10 comments
Open

navi-scripts build just stopped working #83

alexpricedev opened this issue Apr 11, 2019 · 10 comments

Comments

@alexpricedev
Copy link

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?

@jamesknelson
Copy link
Collaborator

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.

@alexpricedev
Copy link
Author

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
Copy link
Author

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!

@jamesknelson
Copy link
Collaborator

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).

@alexpricedev
Copy link
Author

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.

@pomppa
Copy link

pomppa commented Jun 9, 2019

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

@decebal
Copy link

decebal commented Aug 27, 2019

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

@jamesknelson jamesknelson reopened this Aug 28, 2019
@jamesknelson
Copy link
Collaborator

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.

@decebal
Copy link

decebal commented Aug 28, 2019

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 :(

@HAKASHUN
Copy link

HAKASHUN commented Nov 8, 2022

hello! @jamesknelson

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

HAKASHUN@56e073c

This works fine in my environment.

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

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

No branches or pull requests

5 participants