Fix JS Builds on Windows

[Archmonger]

• Fix JS build issues causing Failed to resolve module specifier "react"
• Look into whether channels can be monkey-patched to fix the ChannelsLiveServerTestCase

Full error:
Uncaught TypeError: Failed to resolve module specifier "react". Relative references must start with either "/", "./", or "../".

[Archmonger]

Starting this off as a draft while I investigate what exactly is broken.

[Archmonger]

cc: @rmorshea

Things of note

• I had to manually run npm install --global rollup in order to get rollup to work at all
• Based on this log output MANIFEST.in paths are incorrectly configured
• react does not appear to be automatically resolved by rollup (treating it as an external dependency)

Although I'm not familiar with rollup I'll try to dig into this deeper and see if I can figure out a solution.

---

Here's the output of my pip install -e .. I've excluded lines prior to this that are normal for a pip install.

2021-09-09T02:50:47,594 Installing collected packages: django-idom
2021-09-09T02:50:47,594   Attempting uninstall: django-idom
2021-09-09T02:50:47,596     Found existing installation: django-idom 0.0.1
2021-09-09T02:50:47,598     Uninstalling django-idom-0.0.1:
2021-09-09T02:50:47,945       Created temporary directory: C:\Users\username\AppData\Local\Temp\pip-uninstall-rcoedx6f
2021-09-09T02:50:47,946       Removing file or directory c:\users\username\documents\repositories\django-idom\.venv\lib\site-packages\django-idom.egg-link
2021-09-09T02:50:47,946       Removing pth entries from c:\users\username\documents\repositories\django-idom\.venv\lib\site-packages\easy-install.pth:
2021-09-09T02:50:47,946       Removing entry: c:\users\username\documents\repositories\django-idom\src
2021-09-09T02:50:47,946       Successfully uninstalled django-idom-0.0.1
2021-09-09T02:50:47,949   Running setup.py develop for django-idom
2021-09-09T02:50:47,949     Running command 'c:\users\username\documents\repositories\django-idom\.venv\scripts\python.exe' -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'C:\\Users\\username\\Documents\\Repositories\\django-idom\\setup.py'"'"'; __file__='"'"'C:\\Users\\username\\Documents\\Repositories\\django-idom\\setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' develop --no-deps
2021-09-09T02:50:48,200     building js
2021-09-09T02:50:48,200     building js
2021-09-09T02:50:48,200     building js
2021-09-09T02:50:48,201     running develop
2021-09-09T02:50:48,332     Installing Javascript...
2021-09-09T02:50:48,344     > C:\Program Files\nodejs\npm.CMD install
2021-09-09T02:50:49,429     npm WARN idom-client-react@0.33.2 requires a peer of react@>=16 but none is installed. You must install peer dependencies yourself.
2021-09-09T02:50:49,437     npm WARN idom-client-react@0.33.2 requires a peer of react-dom@>=16 but none is installed. You must install peer dependencies yourself.
2021-09-09T02:50:49,445     npm WARN django-idom-client@0.0.1 No repository field.
2021-09-09T02:50:49,452     npm WARN django-idom-client@0.0.1 No license field.
2021-09-09T02:50:49,466     npm WARN optional SKIPPING OPTIONAL DEPENDENCY: fsevents@2.3.2 (node_modules\fsevents):
2021-09-09T02:50:49,466     npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@2.3.2: wanted {"os":"darwin","arch":"any"} (current: {"os":"win32","arch":"x64"})

2021-09-09T02:50:49,604     audited 24 packages in 0.447s

2021-09-09T02:50:49,617     3 packages are looking for funding
2021-09-09T02:50:49,617       run `npm fund` for details

2021-09-09T02:50:49,617     found 0 vulnerabilities

2021-09-09T02:50:49,639     > C:\Program Files\nodejs\npm.CMD install
2021-09-09T02:50:49,639     > C:\Program Files\nodejs\npm.CMD run build

2021-09-09T02:50:50,234     > django-idom-client@0.0.1 build C:\Users\username\Documents\Repositories\django-idom\src\js
2021-09-09T02:50:50,234     > rollup --config

2021-09-09T02:50:50,393     �[36m
2021-09-09T02:50:50,393     �[1msrc/index.js�[22m → �[1m../django_idom/static/js/django-idom-client.js�[22m...�[39m
2021-09-09T02:50:50,474     'react' is imported by node_modules\idom-client-react\src\components.js, but could not be resolved – treating it as an external dependency
2021-09-09T02:50:50,474     'react' is imported by node_modules\idom-client-react\src\mount.js, but could not be resolved – treating it as an external dependency
2021-09-09T02:50:50,474     'react-dom' is imported by node_modules\idom-client-react\src\components.js, but could not be resolved – treating it as an external dependency
2021-09-09T02:50:50,475     'react-dom' is imported by node_modules\idom-client-react\src\mount.js, but could not be resolved – treating it as an external dependency
2021-09-09T02:50:50,475     'react' is imported by node_modules\idom-client-react\src\json-patch.js, but could not be resolved – treating it as an external dependency
2021-09-09T02:50:50,608     �[32mcreated �[1m../django_idom/static/js/django-idom-client.js�[22m in �[1m216ms�[22m�[39m
2021-09-09T02:50:50,628     > C:\Program Files\nodejs\npm.CMD run build
2021-09-09T02:50:50,628     Successfully installed Javascript
2021-09-09T02:50:50,628     running egg_info
2021-09-09T02:50:50,628     writing src\django_idom.egg-info\PKG-INFO
2021-09-09T02:50:50,629     writing dependency_links to src\django_idom.egg-info\dependency_links.txt
2021-09-09T02:50:50,629     writing requirements to src\django_idom.egg-info\requires.txt
2021-09-09T02:50:50,630     writing top-level names to src\django_idom.egg-info\top_level.txt
2021-09-09T02:50:50,634     reading manifest file 'src\django_idom.egg-info\SOURCES.txt'
2021-09-09T02:50:50,636     reading manifest template 'MANIFEST.in'
2021-09-09T02:50:50,637     warning: manifest_maker: MANIFEST.in, line 1: path 'src/django_idom/static/' cannot end with '/'

2021-09-09T02:50:50,637     warning: manifest_maker: MANIFEST.in, line 2: path 'src/django_idom/templates/' cannot end with '/'

[Archmonger]

The automatic installation of peer dependencies was explicitly removed with npm 3.

As a result I've begun manually installing react and react-dom.

Also since Windows installs of npm don't come pre-bundled with rollup, I've included that too.

[Archmonger]

@rmorshea let me know if this solution is appropriate, or if I need to automating this with rollup.

[rmorshea]

Ok, so I think I modified my path to include ./node_modules/.bin which is where local dependencies like rollup get installed and that's why I'm not having this issue (that or npm does this automatically on unix systems).

With that said, I think the better solution is to use npx which comes bundled with npm>5. That way you can just run npx rollup and it will find the locally installed executable for rollup - no need to install globally.

edit: I think npm adds the local node_module/.bin to your path when executing something from a script in package.json.

[Archmonger]

I'm unfamiliar with NPX.

Would your suggestion involve changing all npm calls within setup.py to instead use npx?

You should have write access to this PR if you want to do work within this branch.

[rmorshea]

To really be sure that this all works on Windows and, more importantly, that it continues to, we should add windows to the set of systems check during CI. You can refer to IDOM's workflow on how to do this. I was having problems though with running tests that relied on the Chrome webdriver on Windows though. If we're able to solve that here, maybe we can port that solution back into the main idom repo so we can run the full test suite on Windows there too.

Sorting out all these CI issue on Windows for this project and the core repo would be a huge win!

[Archmonger]

To really be sure that this all works on Windows and, more importantly, that it continues to, we should add windows to the set of systems check during CI. You can refer to IDOM's workflow on how to do this. I was having problems though with running tests that relied on the Chrome webdriver on Windows though. If we're able to solve that here, maybe we can port that solution back into the main idom repo so we can run the full test suite on Windows there too.

I did do some poking and prodding into potential monkey patches but haven't confirmed anything as a solution.

I did notify the django team @ django/channels#1207 that the issue is WIndows specific.

[rmorshea]

So I think that if you want to invoke rollup manually at the command line, you'll want to do with with npx. However, if you run a script defined in package.json (e.g. npm run build) the node_module/.bin folder will be added to your path automatically. Let me know if that doesn't work though.

Ideally, you should be able to uninstall your global rollup install. Then, start with a fresh environment (delete src/js/node_modules). it should be possible to cd src/js run npm install and then execute npm run build without issue. Though you'll probably need to do that after adding react, react-dom as dependencies in package.json.

These npm run commands (those invoking scripts defined in package.json) are currently being executed in setup.py.

[rmorshea]

I think the issues with the WebDriver in the main IDOM repo are probably independent of channels. So maybe best to unlink reactive-python/reactpy#289 from the issue there?

[Archmonger]

Unlinked the idom issue from the comment on django-channels.

[Archmonger]

Looks like bumping rollup and adding react/react-dom to package.json seems to have fixed windows builds.

Doesn't look like npx was needed to resolve rollup's path.

Also I certainly have no idea how this was building without react before.

[Archmonger]

I'll continue poking around to figure out why tests are broken on Windows. But I don't think a resolution will come easy, since the trace log indicates it's related to how pickling works on Windows.

If I find a solution I'll create a separate PR.

[Archmonger]

Looks like while tests were passing while it was in this branch, after merging tests have failed.

🤔