"Cannot find module 'sharp'" on Windows only after upgrading to 26.0.11
After upgrading to 26.0.11 it seems Sharp isn't being properly bundled or run. None of our config changed except changing win.publisherName to win.signToolOptions.publisherName.
I'm not sure how to go further in investigating this. Any advice appreciated!
Hiya! Can you share a minimum reproducible repo that I can test locally with and/or create a unit test around? Would like to assist but need more information first.
Hey @mmaietta thanks for your reply.
I did find the problem - in files we had this rule, which worked fine in 25.x but not in 26.x (Sharp version didn't change):
!**/node_modules/sharp/install/*
Simply removing this is enough of a fix for us and probably only adds a tiny bit to the package size, but I do wonder why that would now be breaking in 26.x (and seemingly only on Windows.)
Thanks again!
Actually, I must have overlooked something because that's definitely not resolving the issue now...
Hm... I really thought I had this working but every attempt now results in this same issue. I'm not sure if I changed something else or if what I thought fixed it never really did. I'll update soon.
@mmaietta I did notice that there's nothing under app.asar.unpacked from Sharp (or any other Node dependencies.
Yeahhhh, definitely need a minimum repro repo for this unfortunately. The node collector logic was completely migrated to TS from Golang to support pnpm amongst other things, and a migration to electron/asar in order to support electron/fuses integration
Thanks as always for the great work and for your help @mmaietta!
I put together a repro here: https://github.com/Nantris/electron-builder-repro-8970
It requires Yarn v4 (which I think should be used automatically as long as you have corepack enabled?)
Repro steps:
yarn installyarn package- Run executable in
win-unpacked
This isn't an exact copy of our config and may not be 100% correct in every regard, but it should at least output the app.asar.unpacked folder I would think. Alas, it does not.
Well this is odd indeed. Your .asar doesn't have any node modules dir present. What version of electron-builder are you upgrading from? I'm wondering if this is a yarn 4-specific bug
Thanks as always for all you do @mmaietta!
I was wondering if it might be related to Yarn 4 myself.
We upgraded from 25.1.8 and I believe I verified this affects 26.0.0 specifically but I'm not certain now.
It's super strange to me that while Windows fails, Linux builds remain unaffected. I haven't had a chance to test macOS. Maybe there's some sort of path issue that makes this only work on POSIX?
I confirmed macOS is unaffected. So only Windows has the problem.
I also tried setting supportedArchitectures in .yarnrc.yml even though it works in dev, and as expected, no difference.
I am experiencing this issue too, but with a different dependency. this works with 26.0.2 (before the node_module collector change), but not with 26.0.11. I am using yarn 4.7.0 Reported by a windows user, but it looks like it affects all our builds.
If I look inside the app.asar, there is no node_modules folder at all, we have 10+ direct dependencies that should be included there.
I should add a disclaimer that we are using a lightly patched app-builder-lib, but that should only be including more node_modules than usual loose inside the resources directory.
The snippet of our config that I expect could affect this is:
files: ['**/*', 'assets/*'],
extraResources: [
{
from: '../dist',
to: '.',
filter: ['**/*', '!.yarn'],
},
],
@beyondkmp can you please take a look at this when you have a chance? Seems there's an issue specifically on Windows, and it may be specific to yarn 4.
I'm trying to write a unit test from my side, but once corepack is enabled in the docker container to support a yarn4-scoped test, subsequent runs of other unit tests break due to corepack now being enabled.
Confirming that the node module collection logic is not pulling any node modules when using yarn 4, even on macOS dir target. For some reason, npm list -a --include prod --include optional --omit dev --json --long --silent returns an empty dependency tree when running from the cwd, whereas running it from root seems to work correctly
@mmaietta that's very strange! Because our macOS build worked unless I'm crazy.
Edit: Reconfirming macOS built properly with [email protected]
Hmmm, I'm running off latest master
I was able to resolve the dependency collection by changing the rootDir to no longer be the app dir, but the project root dir, however, now JSON.parse fails on parsing the scripts response due to malformed JSON in the npm list CLI output
"concurrently \\"npm run build:main\\" \\"npm run build:renderer\\""
Trying to figure out how to get around this issue now, as at least the dependency tree is being returned correctly. Suggestions welcome as I'm still pretty sick and my mind is an absolute ball of fuzz right now
I'm getting similar problem on 26.0.11 - multiworkspace project, unpacking asar shows there are no node_modules there at all.
@stq, is that also a yarn 4 project?
@stq would you mind putting together a minimum reproducible repo since yours isn't yarn 4? I'd like to create a dedicated unit test for this multi-workspace project as we already have some two-package.json app fixtures, but it seems like we're missing a use case.
@Nantris can you try this patch-package? It works all of a sudden for me on macos target, still need to setup a test env in a windows VM though
patches/app-builder-lib+26.0.11.patch
diff --git a/node_modules/app-builder-lib/out/util/appFileCopier.js b/node_modules/app-builder-lib/out/util/appFileCopier.js
index 79dee3f..3308f61 100644
--- a/node_modules/app-builder-lib/out/util/appFileCopier.js
+++ b/node_modules/app-builder-lib/out/util/appFileCopier.js
@@ -144,7 +144,7 @@ function validateFileSet(fileSet) {
}
/** @internal */
async function computeNodeModuleFileSets(platformPackager, mainMatcher) {
- const deps = await (0, node_module_collector_1.getNodeModules)(platformPackager.info.appDir);
+ const deps = await (0, node_module_collector_1.getNodeModules)(platformPackager.projectDir);
builder_util_1.log.debug({ nodeModules: deps }, "collected node modules");
const nodeModuleExcludedExts = getNodeModuleExcludedExts(platformPackager);
// serial execution because copyNodeModules is concurrent and so, no need to increase queue/pressure
@mmaietta I'm sorry to report it didn't resolve the issue. But macOS always worked for us, so maybe there are two issues here? It's pretty confusing.
Ok that is weird, as it looks like it packaged correctly for me with just that change. Launches just fine too.
@mmaietta you're testing on the master branch, right? Unfortunately I can't get that working here, apparently due to this issue, so I guess we may be testing slightly different setups.
That image is from macOS, right?
@Nantris can you try this patch-package? It works all of a sudden for me on macos target, still need to setup a test env in a windows VM though
patches/app-builder-lib+26.0.11.patchdiff --git a/node_modules/app-builder-lib/out/util/appFileCopier.js b/node_modules/app-builder-lib/out/util/appFileCopier.js index 79dee3f..3308f61 100644 --- a/node_modules/app-builder-lib/out/util/appFileCopier.js +++ b/node_modules/app-builder-lib/out/util/appFileCopier.js @@ -144,7 +144,7 @@ function validateFileSet(fileSet) { } /** @internal */ async function computeNodeModuleFileSets(platformPackager, mainMatcher) { - const deps = await (0, node_module_collector_1.getNodeModules)(platformPackager.info.appDir); + const deps = await (0, node_module_collector_1.getNodeModules)(platformPackager.projectDir); builder_util_1.log.debug({ nodeModules: deps }, "collected node modules"); const nodeModuleExcludedExts = getNodeModuleExcludedExts(platformPackager); // serial execution because copyNodeModules is concurrent and so, no need to increase queue/pressure
This will include all dependencies from projectDir, while the dependencies in appDir should only be a subset of projectDir's dependencies and don't need to be entirely included.
@mmaietta you're testing on the master branch, right? Unfortunately I can't get that working here, https://github.com/yarnpkg/berry/issues/5715, so I guess we may be testing slightly different setups. That image is from macOS, right?
Should be the same setup. That was on a default yarn install from a fresh git clone, then just patched the appFileCopier.js directly within node_modules/app-builder-lib. Built within a Windows arm64 VM
@mmaietta this is my bad, but actually even more confusing. Originally I tested against our project and not against the repro.
The patch works on the repro :)
Does not work on our project :(
Edit 1: Investigating further. If I come across anything I'll update.
Edit 2: I updated our config to match the repro as closely as I can and I can't find any discrepancies, but our real project still fails to bundle node_modules. I guess I need to next test the repro on macOS to see if that fails for me as it did for you @mmaietta, and beyond that I'm at a total loss.
Oh lordy, this is becoming a conundrum n' a half.
I was going to suggest this as an easy fix. Checks the appDir (current behavior) and if no dependencies found, it has a fallback up the tree to the projectDir
export async function getNodeModules(projectDir: string, appDir: string): Promise<NodeModuleInfo[]> {
const collector = await getCollectorByPackageManager(appDir)
let deps = await collector.getNodeModules()
if (!deps.length && appDir !== projectDir) {
deps = await (await getCollectorByPackageManager(projectDir)).getNodeModules()
}
return deps
}
But since it doesn't work for your original/source project, I'm not sure where to go from here either.
Might need @stq to get back to us on a minimum repro project so I can do a deeper dive since this doesn't seem isolated to yarn 4. Might be due to multi-workspace setups, which is my hunch, but I need a local setup to repro and write a unit test for. Otherwise I'm just stabbing in the dark to find a needle in a haystack 😅
I think it may be useful to add a log.warn when there's no deps found to prevent the issue going unnoticed by unsuspecting victims
The repro I provided does indeed fail for me on macOS, whereas my project does not.
Might be due to multi-workspace setups, which is my hunch
Seems very possible, since our project is indeed a Yarn workspace, but all the native deps are at the project root so one wouldn't think it would be the cause, but it's the most promising theory I'd say.
The only other differences I can find are:
- Our build config file is nested in a directory in our real project whereas it's in the project root in the repro, but I don't think there's any plausible way that could make a difference?
- We have additional native dependencies.
I noticed some debug statements in app-builder-lib. I wonder if I can force those to output and try to find any discrepancies between the repro and our real project?
For now I guess we're going to stick with 26.0.2 and pin hopes on an independent repro hopefully pointing to whatever this other issue is.
I noticed some debug statements in app-builder-lib. I wonder if I can force those to output and try to find any discrepancies between the repro and our real project?
Set env var DEBUG=electron-builder and extract the relevant logs maybe? Might be a bit long, so try using markdown to have it a collapsible section
Related note: Once we iron this issue out, I think we'll be able to promote 26.0.12 to latest and see what issues come in. We already have a solid % of downloads on 26.0.2+ (post node-collector migration).
Post-rollout, I'll also begin working on creating a new electron/ repo to migrate our parsing logic there for broader availability to the electron ecosystem. 💪