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

Requiring "solid-js/web" on electron (with nodeIntegration) makes it think I'm on the server #1102

Closed
mauricioszabo opened this issue Jul 5, 2022 · 21 comments
Labels
downstream issue related tools that would use Solid

Comments

@mauricioszabo
Copy link

mauricioszabo commented Jul 5, 2022

Describe the bug

I want to use SolidJS in an Electron. Using require('solid-js/web').isServer returns true, which means I can't use render and other web-related code.

I can work-around by requiring solid-js/web/dist/web.cjs but seems fragile...

Your Example Website or App

https://github.com/mauricioszabo/solid-electron-error

Steps to Reproduce the Bug or Issue

Inside an electron app with nodeIntegration: true try to require solid-js/web. The contents of isServer is true

Expected behavior

I expect require('solid-js/web') to always require "web" stuff, or at least have something like require('solid-js/browser') that always points to browser, and require('solid-js/server') that always points to server

Screenshots or Videos

No response

Platform

  • OS: Ubuntu 12.10
  • Browser: Electron 12.0

Additional context

No response

@edemaine
Copy link
Contributor

edemaine commented Jul 5, 2022

This is the normal behavior in Node. Currently, on Node, importing solid-js itself gives you an SSR build which has no reactivity. So solid-js/web can't "just work".

Importing directly from solid-js/web/dist/web.cjs or solid-js/web/dist/web.js (ESM) is one current workaround. Another way to set a Node condition of "browser" (via -C or NODE_OPTIONS), but I'm not sure whether this would interfere with Electron.

I agree that this situation is pretty annoying for using reactivity on Node, but we need an actual proposal for behavior that still enables SSR work.

@mauricioszabo
Copy link
Author

The problem is that I can't use templating libs like h because the also require solid-js/web :(

@mauricioszabo
Copy link
Author

But I actually don't get it - why can't Solid have two different requires, one for SSR and one for Browser? Why they need to be the same path? Is there a reason for that?

@edemaine
Copy link
Contributor

edemaine commented Jul 5, 2022

I think the reason is "being isomorphic". Usually you have one file define a component, and that file needs to be loaded on both client and server for SSR. And it needs to import ... "solid-js" and do different things on each side.

I think Node conditions are a good way. I just don't like that you need to set a condition of "browser" to get server-side reactivity. Personally I'd prefer if you had to set a condition of "ssr" to opt-in to SSR, and the default Node import would just give you reactivity out of the box. But we might need to find something more backward compatible, like a "reactive" condition to tell Node to just load the client build. (I assume you don't need SSR in an Electron context.)

@mauricioszabo
Copy link
Author

I think I may be missing something: this code does not work on NodeJS, for example (pure node, no Electron):

const h = require('solid-js/h');
const s = require('solid-js/web');
function A() { 
  return h('div', {}, 'Hello, world!') 
}

s.renderToString(A)

It fails with "Uncaught TypeError: Cannot read property 'has' of undefined". I found that the trouble is that solid-js/h actually loads solid-js/web and then uses web.SVGElements - which is undefined...

Which is to say: as of now, h(...) is not working, so the API does not seem "isomorphic" really... I'm not sure how to proceed from here...

@edemaine
Copy link
Contributor

edemaine commented Jul 5, 2022

It seems that Electron might define a Node condition "electron". If true, perhaps Solid could be modified to export the client build in this case, as I can't imagine it being useful to do SSR in Electron?

@Genuifx
Copy link
Contributor

Genuifx commented Jul 6, 2022

You can split your UI and backend in electron, simulating the network's C/S artfs. then use solid at your UI part.

@ryansolid
Copy link
Member

Personally I'd prefer if you had to set a condition of "ssr" to opt-in to SSR, and the default Node import would just give you reactivity out of the box. But we might need to find something more backward compatible, like a "reactive" condition to tell Node to just load the client build. (I assume you don't need SSR in an Electron context.)

Me too but I wanted it to be consistent with legacy. Pre esm and export conditions having the browser field was the pattern. Which makes sense. Package.json was built for npm, ie node.

@mauricioszabo
Copy link
Author

So, after talking in Discord, I decided to make some experiments with a greenfield Electron app. It actually doesn't work, even if I "patch" solid/h to require the browser target - the UI renders, but it does not react in any way: https://github.com/mauricioszabo/solid-electron-error

To run, just install packages and run with npm start. The relevant code is on the view.js file.

@mauricioszabo
Copy link
Author

Also, updating what we were discussing on Discord here: I'm trying to use SolidJS to make some changes in an Atom plug-in, so I can't really change NODE_OPTIONS or things like that (because that's handled by Atom itself). I also can't use JSX because the plug-in is written in ClojureScript, and there's no way to return a JSX content inside ClojureScript, unfortunately...

@ryansolid
Copy link
Member

So, after talking in Discord, I decided to make some experiments with a greenfield Electron app. It actually doesn't work, even if I "patch" solid/h to require the browser target - the UI renders, but it does not react in any way: https://github.com/mauricioszabo/solid-electron-error

I haven't pulled this down but if I were to guess it's because solid-js/web references solid-js directly. So even with the client version of web we are getting the server version of solid's core. The server version is non-reactive which would explain no updates. Short of using conditions, you'd need to alias all the references. This is how we landed on conditions in the first place. Trying to get this stuff to play nice for isomorphic SSR was a pain. As it turns out going the opposite direction is similarly so.

@mauricioszabo
Copy link
Author

you'd need to alias all the references
How do I do that?

Also, is there a way to make Electron behave as if it's always a web version? I'm not sure why one would want to use SSR on Electron...

@ryansolid
Copy link
Member

you'd need to alias all the references

How do I do that?

Depends on the bundler being used.

Also, is there a way to make Electron behave as if it's always a web version? I'm not sure why one would want to use SSR on Electron...

But this I think we can do. It has to be how others solve this because we aren't the only ones with this issue for sure. Maybe not export conditions but stuff like the browser field predate this. Targets like workers sometimes need similar consideration. My guess is electron sets node which is why we don't get the client version which is the default or it uses the main field export directly which would also do this. The latter I imagine is more likely so, let me see what others do.

Hmm.. this is dated, using webpack with aliasFields https://medium.com/@dylanavery720/marko-marko-3-electron-webpack-84e3edbe90cc

This is documentation from webpack on how to set conditions: https://webpack.js.org/configuration/resolve/#resolveconditionnames. In which case we'd want browser. In the list.

Sorry lack of familiarity with electron specifically and the build process. But most bundlers have the ability to set these things.

@edemaine
Copy link
Contributor

edemaine commented Jul 6, 2022

Me too but I wanted it to be consistent with legacy. Pre esm and export conditions having the browser field was the pattern. Which makes sense. Package.json was built for npm, ie node.

Yeah, makes sense. Another option would be to publish as a totally different NPM package. solid-js could remain the SSR-capable version, while @solid.js/solid or something like that (@solid.js/reactive? @solid.js/nossr?) could be the always-reactive never-SSR version. Then we wouldn't have to rely on Node conditions... I think this would be great for using Solid reactivity into Node in general (without GUIs) and it would fix this particular problem easily too.

@ryansolid
Copy link
Member

@edemaine That'd be fine for the reactive stuff. Like people want to just use reactivity.

But like this case here we actually want web as well. So references in web have to change as well. So I don't think that actually helps a ton here, or testing envs or any of the typical Solid workflows.

@edemaine
Copy link
Contributor

edemaine commented Jul 6, 2022

Sorry, to clarify, I meant you could import ... from:

  • "@solid.js/solid" (Solid core)
  • "@solid.js/solid/store"
  • "@solid.js/solid/web"
  • "@solid.js/solid/h"
  • etc.

All the subdirectories would work as before, but with only the browser (and dev) exports that solid-js currently has. You'd just npm install @solid.js/solid instead of npm install solid-js.

Of course, everything would break down if you installed both packages solid-js and @solid.js/solid, because then you'd get two copies of Solid core. Oh, and there's the rub: if you use a library that imports solid-js, you'd get exactly this. So while this approach would work in isolation, it won't work with any code that uses libraries. ☹

@mauricioszabo
Copy link
Author

mauricioszabo commented Jul 6, 2022

But this I think we can do

If possible, it would help me a lot. My case is quite complicated because I'm using Solid with ClojureScript. It uses closure compiler, which as far as I know don't handle NPM too well, but I'm running over Shadow-CLJS which do handle rewriting of NodeJS requires. I tried some configs, but probably did something wrong because I could not make it work...

@ryansolid
Copy link
Member

Oh I see so this is more than just straight Electron. Because that article I linked from 2017 I imagine would still work today. But if webpack isn't even involved and we we are talking about some bundling/building that doesn't have any of these sort of aliasing. Looking at common config though it looks like renderer is set with browser target at least in starters like: https://github.com/ahonn/shadow-electron-starter/blob/master/shadow-cljs.edn

So I would have hoped this would just find the browser fields. Hmm.

@carlosqsilva
Copy link

don't know what bundler are you using, but I put together this esbuild plugin for my electron application with solid-js, you could use this article (shadow-cljs with webpack) as a reference, and make it works with esbuild, hope it helps

const { parse } = require("path");
const { readFile } = require("fs/promises");
const { transformAsync } = require("@babel/core");
const solid = require("babel-preset-solid");
const ts = require("@babel/preset-typescript");

const defaultOptions = {
  hydratable: true,
  generate: "dom",
};

const regexMap = {
  "solid-js": /(?<=solid-js\/).*$/gm,
  "solid-js/web": /(?<=solid-js\/web\/).*$/gm,
  "solid-js/store": /(?<=solid-js\/store\/).*$/gm,
};

/** @type {() => import('esbuild').Plugin} */
function solidPlugin(options) {
  const pluginOptions = Object.assign({}, defaultOptions, options);

  return {
    name: "esbuild:solid",

    setup(build) {
      build.onResolve({ filter: /solid-js/ }, ({ path, resolveDir }) => {

        let newPath = require.resolve(path, {
          paths: [resolveDir],
        });

        if (path === "solid-js/web") {
          newPath = newPath.replace(regexMap[path], "dist/web.js");
        } else if (path === "solid-js/store") {
          newPath = newPath.replace(regexMap[path], "dist/store.js");
        } else {
          newPath = newPath.replace(regexMap[path], "dist/solid.js");
        }

        return {
          path: newPath,
        };
      });

      build.onLoad({ filter: /\.tsx$/ }, async (args) => {
        const source = await readFile(args.path, { encoding: "utf-8" });

        const { name, ext } = parse(args.path);
        const filename = name + ext;

        const { code } = await transformAsync(source, {
          presets: [[solid, pluginOptions], ts],
          filename,
          sourceMaps: "inline",
        });

        return { contents: code, loader: "js" };
      });
    },
  };
}

module.exports = { solidPlugin };

``

@ryansolid
Copy link
Member

ryansolid commented Sep 22, 2022

This section suggests webpack uses electron condition with either browser or node so we should already be good here: https://webpack.js.org/guides/package-exports/#target-environment.

Would love an update or confirmation. But looks like we should already be setup right.

@ryansolid
Copy link
Member

Closing as stale.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
downstream issue related tools that would use Solid
Projects
None yet
Development

No branches or pull requests

5 participants