Properly filter files that exist outside of the app directory (like hoisted modules) (fixes #2942)

[jlongster]

I figured out the problem in #2942.

This isn't a Windows-specific issue like I first thought; I just hadn't tested my new build pipeline on macOS yet.

electron-builder supports yarn workspaces, where modules have been hoisted into a root repo. The electron app exists in a subpackage like packages/desktop-electron, while almost all dependencies have been hoisted up into a top-level node_modules.

The problem is this piece of code in createFilter:

let relative = it.substring(srcWithEndSlash.length)

The problem is it assumes it, which is a path, exists below srcWithEndSlash. This may not be the case with hoisted modules. it may be ~/project/node_modules/lib/foo.js while srcWithEndSlash is ~/project/packages/desktop-electron/. The result is that it will be erroneously cut off, since it will slice off the full length of srcWithEndSlash.

The result is you end up with partial paths, which then might falsely match filters. In my case, a dependency had a dist folder, and the node_modules/lib/dist/file.js path happened to get sliced into dist/file.js which matches the !dist{,/**/*} filter which makes it not include that file. (That filter is attempting to not include the dist folder generated by electron-builder). The result is the dist folder of an internal dependency is missing.

This is probably not the right solution. I also haven't written TS before, but this is the code that works if I patch electron-builder in node_modules manually. I'm happy to help improve this and get some kind of fix in.

[jlongster]

I didn't really explain the behavior of this change. We don't really know what the true "root" of the project is since a yarn workspace could live anywhere within the project. This takes a relaxed approach and, if a file outside the electron app is found, it will return whatever its path has in common with the path of the electron app. So if the electron app exists at ~/app/packages/electron-app and a file is passed in ~/app/node_modules/lib/foo.js, the path that it will make is node_modules/lib/foo.js.

[jlongster]

Alright, I got it working in my own repo and made sure it compiles with TS.

[develar]

Thanks for PR! Even if it is not fully correct, such PR is the only way to force me to fix such bugs since it is a challenge :)

[develar]

• copyHoistedNodeModules function must not filter dist and other app-only related default patterns at all.

[jlongster]

Let me know the right way you think this should be fixed. Don't want to force you to do it :) I have to use a custom fork now though so I'd love to get rid of that.

[develar]

Fix will be tomorrow morning CET. Thanks a lot for your clear and full explanation.

[jlongster]

Thanks! To be clear: I also need the ability to exclude files from the root of the project. Yarn workspaces have installed a link to my other subpackage "loot-core" in the root node_modules, so with my PR, I can exclude a directory inside of it by adding "!node_modules/loot-core/src{,/**/*}" to the files option. Normally these patterns are run against the electron app dir, which in this case would be packages/electron-desktop, but files outside of it are a special case.

Just want to make sure I can still somehow write files patterns` to match files outside of the electron app dir.

[develar]

Ideally, should be an another option to specify patterns for node modules, but for now we simply use only exclude patterns from main patterns (and base always node_modules). So, your case will be still supported.