MarshallOfSound / flora-colossus

Walk your node_modules folder
6 stars 9 forks source link

support pnpm module layout #24

Open MichaelBelousov opened 3 years ago

MichaelBelousov commented 3 years ago
MichaelBelousov commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

MichaelBelousov commented 3 years ago

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

MichaelBelousov commented 2 years ago

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

MichaelBelousov commented 2 years ago

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 commented 1 year ago

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

MichaelBelousov commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 :)