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

support pnpm module layout #24

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

MichaelBelousov
Copy link

  • this is hopefully the start of introducing link-aware module resolution to several electron libraries with the end goal of giving electron-forge support for using the pnpm package manager (as well as theoretically any other package manager, except I haven't looked into whether the new technique I added supports Plug n' Play)
  • please take a look at the way I use require.resolve, given the problems with require.resolve respecting package.json#exports, it may be better to write a custom link-aware resolver for this situation...

@MichaelBelousov MichaelBelousov changed the title work on pnpm support pnpm module layout Sep 24, 2021
@MichaelBelousov
Copy link
Author

MichaelBelousov commented Sep 24, 2021

I would wait to merge this until I have electron-forge with full pnpm support, but I wanted to initiate some discussion so I made this PR as I move on to adding support for pnpm in galactus

@MichaelBelousov
Copy link
Author

Update: I have locally got it working mostly EXCEPT that I seem to have run into an issue with link-containing paths on windows which I'm investigating

@MichaelBelousov
Copy link
Author

It seems like the only way to support pnpm in the electron dev tools stack would be to emulate non-lexical traversal of parent-directory path specifiers on windows (..) in at least the node-glob package if not fs.

@MichaelBelousov
Copy link
Author

MichaelBelousov commented Oct 19, 2021

Let me clarify the previous comment. There are two things that would be needed:

  • supplementing the usage of node-glob with some kind of deep traversal option that support's pnpm's virtual store directory graph.
  • adding emulation of non-lexical traversal of parent directory specifiers (..) in paths on windows, in order to, for example, asar-unpack a nested dependency... although I need to seek clarity on this

@MichaelBelousov
Copy link
Author

The above mentioned requirements have been worked-around, they should no longer be necessary

@MichaelBelousov
Copy link
Author

MichaelBelousov commented Dec 8, 2021

TODO: use a package like resolve instead of require.resolve to get around having to parse errors from difficult package.json exports

@MichaelBelousov
Copy link
Author

I now just use a prebuilt asar and plan on open sourcing the package I use to make it, but may need to look into helping electron-forge take prebuilt asars (because it doesn't)

@GitMurf
Copy link

GitMurf commented Aug 30, 2023

Did this ever end up working for pnpm? Looks like it was on the right track but haven't heard anything recently. Thanks!

@MichaelBelousov
Copy link
Author

MichaelBelousov commented Aug 30, 2023

Did this ever end up working for pnpm? Looks like it was on the right track but haven't heard anything recently. Thanks!

Yes but it's much better to use a bundler like webpack or esbuild. I'll open source my solution if you really want it but prefer bundling

@GitMurf
Copy link

GitMurf commented Aug 30, 2023

@MichaelBelousov thank you for the quick response! When you say a bundler do you just mean like vite (via rollup)? I currently use vite with electron-forge. The problem is during the packaging process of forge.

@MichaelBelousov
Copy link
Author

@MichaelBelousov thank you for the quick response! When you say a bundler do you just mean like vite (via rollup)? I currently use vite with electron-forge. The problem is during the packaging process of forge.

Yes like vite, roll-up, esbuild. You should just disable ASAR and use those so to avoid the problems that ASAR solves

@MichaelBelousov
Copy link
Author

It's been a while since I set up a forge project but iirc you have to disable ASAR and configure the ignores to ignore everything including node_modules except the bundler output

@GitMurf
Copy link

GitMurf commented Aug 30, 2023

@MichaelBelousov thank you for the quick response! When you say a bundler do you just mean like vite (via rollup)? I currently use vite with electron-forge. The problem is during the packaging process of forge.

Yes like vite, roll-up, esbuild. You should just disable ASAR and use those so to avoid the problems that ASAR solves

We build for windows and Mac and in the past the electron team has always recommended still to use ASAR particularly for windows and some of its nasty file path length issues... even when bundled. Do you think that is unnecessary these days with Vite?

@GitMurf
Copy link

GitMurf commented Aug 30, 2023

It's been a while since I set up a forge project but iirc you have to disable ASAR and configure the ignores to ignore everything including node_modules except the bundler output

I am doing exactly that actually :) appreciate the insight and confirming I was at least on the right track ;) haha. But the problem is that I have a native module (Realm DB) that has to be externalized so I need to separately bundle just Realm node module... BUT the problem is that means I also need to include all of Realms dependencies which is why I was using colossus to gather all the nested dependencies for Realm and then ignore all the rest (like you suggested).

I guess I would love your opinion on whether there is a better way that doesn't require grabbing realms nested dependencies? I'm open to any and all feedback / ideas and take no offense if you tell me I am doing something stupid 🤣 and can solve this a simpler way :)

Thanks again for your help!

@MichaelBelousov
Copy link
Author

MichaelBelousov commented Aug 30, 2023

We build for windows and Mac and in the past the electron team has always recommended still to use ASAR particularly for windows and some of its nasty file path length issues... even when bundled. Do you think that is unnecessary these days with Vite?

When I asked the electron forge maintainer about contributing my changes, they said use a bundler. My take is electron these days is a free for all so the recommendations have gotten stale.
If you use a bundler you avoid the windows deep path and file system slowness problems so long as you don't keep too many things outside your bundle.

I deliver an electron application on Windows with a large native component that has to be outside our bundle and it works fine on modern windows without ASAR. We could try to make our bundle tighter but really it's fine these days to just bundle without ASAR, especially if you're not using any large native components and I'd be surprised if you're using a larger native component than us... Ours is huge.

@GitMurf
Copy link

GitMurf commented Aug 30, 2023

I suppose if my issue only is for a single native module (realm) I may just be better off writing my own script to grab realm's dependencies from package.Json and then knowing I am using pnpm I know where to check for the dependencies and can just do a simple recursive grabbing of realms nested dependencies knowing I don't have to support corner cases or generalized usage it is probably fairly straightforward... thoughts?

@MichaelBelousov
Copy link
Author

MichaelBelousov commented Aug 30, 2023

I suppose if my issue only is for a single native module (realm) I may just be better off writing my own script to grab realm's dependencies from package.Json and then knowing I am using pnpm I know where to check for the dependencies and can just do a simple recursive grabbing of realms nested dependencies knowing I don't have to support corner cases or generalized usage it is probably fairly straightforward... thoughts?

No it sounds like your approach is reasonable. It's not an easy task unfortunately.
If you have the time, the ideal solution is you use esbuild or rollup or something with (custom?) plugins configured to correctly analyze the native dependency (realm) and its load point, treat as external (see esbuild and webpack docs) the addon (dll) file itself and separately include it in your bundle output as a second file, while all other javascript including the addon's dependencies are sucked into your bundle.

Now how simple/possible this all is depends on that dependency. For ours it is difficult, so our native addon's javascript dependency tree we do not bundle and we risk bad installations on older windows, as well as non-ideal performance, but we may remedy that eventually. Our addon even needs separate config files, but ideally we'd compile those external configuration files into our the dll/so itself in its build process so we could just need the two files, the addon dll and the js bundle that loads it and does everything else. But that's enough about our woes, just an example of potential issues with the ideal asar-less setup.

@GitMurf
Copy link

GitMurf commented Aug 30, 2023

@MichaelBelousov thanks for all your feedback and help! I'll have to take it back and chew on it a bit and decide the best path for us :)

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.

2 participants