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

package.json: require newest engine to get better touch gesture handling #72

Merged
merged 3 commits into from
Feb 15, 2024

Conversation

pkgw
Copy link
Contributor

@pkgw pkgw commented Feb 9, 2024

Also a general update of deps.

@pkgw
Copy link
Contributor Author

pkgw commented Feb 13, 2024

Build error here seems to have something to do with the latest version of Node.js. I'll try to dig into it a bit.

@pkgw
Copy link
Contributor Author

pkgw commented Feb 13, 2024

Writing down some more info about the error since this is looking like it will take some time to figure out.

The proximate error is during our nuxt build step:

ERROR  RollupError: Could not resolve "./ChainRec" from "./ChainRec?commonjs-external"

(There's also a lot of noise about __PURE__ annotations, seemingly due to naive-ui, but those appear to be just warnings. Also, sometimes the flagged module changes, but it's always in fp-ts — presumably some work is being done in parallel and it's random as to which module gets flagged for the error.)

This has to do with fp-ts. It appears to be some issue related to CJS vs. MJS modules, and fp-ts (and the related io-ts) do package themselves in a somewhat unusual way regarding this. This problem is also manifesting itself only with newer versions of Node.js. I see it with 20.11.0, but not with 18.12.1.

We use fp-ts and io-ts in the apis.ts file:

import { isLeft } from "fp-ts/lib/Either.js";
import * as t from "io-ts";
import { PathReporter } from "io-ts/lib/PathReporter.js";

These imports are for older-style CJS files. If I change them to explicitly point to the ES6 versions, the nuxt build works, but with some worrisome complaints:

node_modules/fp-ts/es6/function.js (1:21) The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten
# and more similar

And more importantly, the nuxt dev stops working. It seems to build fine, but we get errors indicating that the server seems to be looking for CJS rather than MJS:

(node:55830) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.

[...]

[nuxt] [request error] [unhandled] [500] Cannot use import statement outside a module
  at internalCompileFunction (node:internal/vm:77:18)  
  ...
  at cjsLoader (node:internal/modules/esm/translators:356:17)

[...]

[nitro] [unhandledRejection] /a/wwt/sw/wwt-constellations-frontend/node_modules/fp-ts/es6/Either.js:1
import { getApplicativeMonoid } from './Applicative';
^^^^^^

SyntaxError: Cannot use import statement outside a module

Both of these import syntaxes for fp/io-ts are discouraged. Instead we're supposed to use:

import { isLeft } from "fp-ts/Either";
import * as t from "io-ts";
import { PathReporter } from "io-ts/PathReporter";

Unfortunately, this leads to the same nuxt build errors as the /lib/ style imports.

@pkgw
Copy link
Contributor Author

pkgw commented Feb 13, 2024

So far gcanti/io-ts#688 contains the most helpful discussion that I've seen about these kinds of problems. There are some workarounds posted there, but it seems like it might be better to port to Effect. There don't seem to be official porting docs, but this blog post is a start. A port seems plausible since our use of fp/io-ts is fairly constrained and un-fancy.

Here's an Effect Schema getting-started doc.

As hoped, this seems to fix our build issues. And the port is pretty
straightforward; nearly doable with search-and-replace. Not yet
extensively tested but it seems to be working ...
@pkgw
Copy link
Contributor Author

pkgw commented Feb 13, 2024

@Carifio24 OK, I've logged some information about the build error here, which I think I've solved by migrating from fp-ts/io-ts to Effect-ts. Would you have a chance to test out this branch a bit on your machine and help check whether everything seems to be working OK with the change? And that you can build it, etc.

@Carifio24
Copy link
Member

Carifio24 commented Feb 14, 2024

@pkgw This seems to mostly work for me, except I'm seeing a 500 on the scene editing page. The error doesn't tell me much at first glance:

[nuxt] error caught during app initialization Error
    at createError (index.mjs:199:15)
    at createError (error.js:45:68)
    at showError (error.js:21:21)
    at eval (router.js:201:109)
    at fn (nuxt.js:178:44)
    at Object.runWithContext (runtime-core.esm-bundler.js:4107:18)
    at callWithNuxt (nuxt.js:181:24)
    at eval (nuxt.js:50:54)
    at EffectScope.run (reactivity.esm-bundler.js:87:16)
    at Object.runWithContext (nuxt.js:50:44)

Edit: After a bit more testing, I think this was because my local Keycloak Docker instance wasn't running, so that's just a problem on my end. Looks all good now.

@pkgw pkgw merged commit bcf171f into WorldWideTelescope:master Feb 15, 2024
3 checks passed
@pkgw pkgw deleted the update branch February 15, 2024 14:59
@pkgw
Copy link
Contributor Author

pkgw commented Feb 15, 2024

Thanks for checking it out! Let's see how it goes.

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

Successfully merging this pull request may close these issues.

None yet

2 participants