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

Node modules not imported accurately compared to local, missing files. #7964

Closed
12Jeef opened this issue Jan 4, 2024 · 7 comments · Fixed by #8043
Closed

Node modules not imported accurately compared to local, missing files. #7964

12Jeef opened this issue Jan 4, 2024 · 7 comments · Fixed by #8043

Comments

@12Jeef
Copy link

12Jeef commented Jan 4, 2024

  • Electron-Builder Version: 24.9.1

  • Node Version: 18.15.0

  • Electron Version: 28.1.1

  • Electron Type (current, beta, nightly): current

  • Target: MacOS (Ventura M2)

electron-builder is not accurately importing the ThreeJS module into the node_modules directory. I have tried many versions of ThreeJS, but it has no effect on the packaged app. Below is a side-by-side comparison of the difference in the three module located in node_modules. Above is node_modules/three unpacked from the app.asar located in the Resources of the MacOS application. Below is the module as it looks like from local development, not packaged whatsoever.

Screenshot 2024-01-04 at 10 55 59 AM

As you can see, it is missing the examples directory, which contains many crucial files needed for the application to work, such as OrbitControls, etc. Is there any reason electron-builder is not including the examples directory? I have also tried including ThreeJS directly in package.json using the files option, but that seems to have no effect. Here is the entire build configuration:

"appId": "org.app.id",
"copyright": "Copyright © 2024",
"npmRebuild": false,
"publish": [],
"files": [
	"node_modules/three/**/*",
	"node_modules/ionicons/**/*",
	"src/**/*",
	"docs/**/*",
	"package.json",
	"README.md"
],
"fileAssociations": [],
"mac": {
	"target": "dmg",
	"icon": "src/assets/app/icon.icns",
	"notarize": false
},
"linux": {
	"target": [
		"AppImage",
		"snap",
		"flatpak",
		"deb",
		"rpm",
		"pacman"
	],
	"icon": "src/assets/app/icon.png",
	"category": "Utility"
},
"flatpak": {
	"runtimeVersion": "22.08",
	"baseVersion": "22.08"
},
"win": {
	"target": "nsis",
	"icon": "src/assets/app/icon.ico"
}

Some more info: IonIcons, another module I am using, seems to import entirely fine; the packaged and unpackaged versions in node_modules look entirely the same.

@12Jeef 12Jeef changed the title Node modules not imported accurately compared to NPM, missing files. Node modules not imported accurately compared to local, missing files. Jan 4, 2024
@12Jeef
Copy link
Author

12Jeef commented Jan 4, 2024

I am currently resorting to copying over the entire three module out of node_modules into another directory, then importing that instead, but this method is more tedious and I'd prefer not to do it this way.

@mikaelcolboc
Copy link

mikaelcolboc commented Jan 9, 2024

I have the exact same issue.
I think it's weird of three.js to include all their addons inside the "example" directory.
It's misleading and feels wrong. Maybe that's why electron skips that folder. Might be more of a three.js issue than electron.

@mmaietta
Copy link
Collaborator

So it is indeed part of the default excludes in electron-builder.

I agree with @mikaelcolboc that it might not be best for three.js to have addons in the example directory, but I'm wondering if we provide a way to disable default excludes or at least allow electron-builder files to effectively bypass base restrictions?

@mikaelcolboc
Copy link

Thanks @mmaietta for confirming that! So what's the next step? I would address it here for now. Adding an option to disable or have a custom topLevelExcludedFiles set seems reasonable.
I'm happy to give it a shot. I'd like to package my app asap.
I would still submit a ticket on three.js to have them "fix" that example folder, if it hasn't been requested yet. But it might be a significant change that could take time.

@mmaietta
Copy link
Collaborator

So I'm hesitant to add another property as I think there may be an intrinsic issue with how files regex is being performed. Theoretically, I think the default exclusions should be able to be overwritten directly in the files configuration. I feel like I've seen a few other Github issues hit a similar issue with some files not being included even when specified, albeit I can't recall them off the top of my head.

Can you share a minimum reproducible repo for this issue? I can debug it further locally and probably add some unit tests to cover this use case.

@mikaelcolboc
Copy link

Thanks, I just created a private demo repo electron-builder-threejs-bug-example and added you to it.

@mmaietta
Copy link
Collaborator

mmaietta commented Feb 9, 2024

Created PR #8043 and added unit tests for this use case. Now you can leverage onNodeModuleFile hook to explicitly force certain paths to be included in the package (if returning true), otherwise it fallbacks to default logic
For your use case:

onNodeModuleFile: filePath => {
  // Force include this directory in the pakage
  return filePath.includes("node_modules/three/examples")
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants