Fix import path in React.Basic.DOM.Server JS

[kaol]

The string in import statement in Server.js is a path and it requires the file suffix as well.

[pete-murphy]

It does seem that the file extension is required here.

❯ spago repl                        
[1 of 1] Compiling React.Basic.DOM.Server
PSCi, version 0.15.0
Type :? for help

> import Prelude       
> import React.Basic.DOM.Server
> renderToString mempty
node:internal/process/promises:279
            triggerUncaughtException(err, true /* fromPromise */);
            ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/Users/pete/Code/purescript/purescript-react-basic-dom/node_modules/react-dom/server' imported from /Users/pete/Code/purescript/purescript-react-basic-dom/.psci_modules/React.Basic.DOM.Server/foreign.js
Did you mean to import react-dom/server.js?
    at new NodeError (node:internal/errors:372:5)
    at finalizeResolution (node:internal/modules/esm/resolve:437:11)
    at moduleResolve (node:internal/modules/esm/resolve:1009:10)
    at defaultResolve (node:internal/modules/esm/resolve:1218:11)
    at ESMLoader.resolve (node:internal/modules/esm/loader:580:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:294:18)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:80:40)
    at link (node:internal/modules/esm/module_job:78:36) {
  code: 'ERR_MODULE_NOT_FOUND'
}

After these changes, I see no errors

> :r                   
[1 of 1] Compiling React.Basic.DOM.Server
> renderToString mempty
""

[mjrussell]

Huh that's very weird. I can work on a hotfix release tomorrow. I'm only on my phone but any idea why this isn't being picked up by the GitHub actions?

[pete-murphy]

I'm still not sure why this is, but it seems like there is a difference between how Node resolves CJS modules vs ESM modules:

According to the Node ESM docs

Including the file extension is only necessary for packages without an "exports" field.

So CommonJS import without extension succeeds

❯ echo "const { renderToString } = require('react-dom/server'); console.log(renderToString('example'))" | node
example

and ESM import with extension succeeds

❯ echo "import { renderToString } from 'react-dom/server.js'; console.log(renderToString('example'))" | node --input-type=module
example

but ESM without extension fails

❯ echo "import { renderToString } from 'react-dom/server'; console.log(renderToString('example'))" | node --input-type=module
node:internal/process/esm_loader:94
    internalBinding('errors').triggerUncaughtException(
                              ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/Users/pete/Code/purescript/purescript-react-basic-dom/node_modules/react-dom/server' imported from /Users/pete/Code/purescript/purescript-react-basic-dom/[eval1]
Did you mean to import react-dom/server.js?
    at new NodeError (node:internal/errors:372:5)
    at finalizeResolution (node:internal/modules/esm/resolve:437:11)
    at moduleResolve (node:internal/modules/esm/resolve:1009:10)
    at defaultResolve (node:internal/modules/esm/resolve:1218:11)
    at ESMLoader.resolve (node:internal/modules/esm/loader:580:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:294:18)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:80:40)
    at link (node:internal/modules/esm/module_job:78:36)
    at processTicksAndRejections (node:internal/process/task_queues:96:5) {
  code: 'ERR_MODULE_NOT_FOUND'
}

If I add an exports field to node_modules/react-dom/package.json like

  "exports": {
    ".": "./index.js",
    "./server": "./server.js"
  }

then ESM without extension works

❯ echo "import { renderToString } from 'react-dom/server'; console.log(renderToString('example'))" | node --input-type=module
example

[kaol]

Huh that's very weird. I can work on a hotfix release tomorrow. I'm only on my phone but any idea why this isn't being picked up by the GitHub actions?

" 1 workflow awaiting approval First-time contributors need a maintainer to approve running workflows."

[mjrussell]

Sorry I meant why the existing checks didn't catch this error, since Pete was able to reproduce it so easily. I can take a look at them in the morning. Thanks for this PR

[kaol]

I can't say that I understand the ES6 module system all that well. After some combination of yarn upgrades and yarn installs, the current version in master started working with our project and my version broke with

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './server.js' is not defined by "exports" in /home/kaol/src/affresco/apps/mosaico/node_modules/react-dom/package.json imported from /home/kaol/src/affresco/apps/mosaico/output/React.Basic.DOM.Server/foreign.js

I'd just close my PR but since you reproduced my original issue, I don't know what's up with this then. I'd advise against using my PR at least.

[pete-murphy]

why the existing checks didn't catch this error

It seems like spago build doesn't catch bad import paths on the FFI side. Like export * as broken from "module-doesnt-exist" would be a runtime error if broken is called from PS. So a test file would have caught it I think. Not sure what to make of the error disappearing after yarn updates 🤔

[pete-murphy]

Seems there was an exports field added recently: https://github.com/facebook/react/pull/23252

[mjrussell]

It looks like the problem is probably due the fact that the current package.json expects ^17.0.0 of react-dom and that export was added in ^18.0.0. This is a bit tricky, I think the correct thing would be to fix this using the change in the PR, but it sounds like that will break for 18, then do a new version for React 18 (most of the react-basic does not yet support React 18)

[mjrussell]

This would be my recommendation - https://github.com/lumihq/purescript-react-basic-dom/pull/39. Merge that and release as 5.0.1, then we can do a 6.0.0 for React 18

[mjrussell]

5.0.1 released - https://github.com/lumihq/purescript-react-basic-dom/releases/tag/v5.0.1.

Thanks for reporting this!