fix(plugin-vite): unpack native modules #3934

[dmazzella]

• I have read the contribution documentation for this project.
• I agree to follow the code of conduct that this project follows, as appropriate.
• The changes are appropriately documented (if applicable).
• The changes have sufficient test coverage (if applicable).
• The testsuite passes successfully on my local machine (if applicable).

Include native module packages in asar for AutoUnpackNativesPlugin compatibility

[MarshallOfSound]

Can you explain the case where this doesn't work / present a test case. Unclear what bug is being solved here, the plugin definitely works in it's current form so I'm interested in the specific edge case where it doesn't?

[dmazzella]

Can you explain the case where this doesn't work / present a test case. Unclear what bug is being solved here, the plugin definitely works in it's current form so I'm interested in the specific edge case where it doesn't?

this PR resolve #3934

[dmazzella]

@MarshallOfSound Did you get a chance to see the #3934 and my PR?

[erickzhao]

@dmazzella I think it'd be good to add a test case showing what this PR fixes. It'll also help us catch regressions in the future. 🙇

[dmazzella]

@dmazzella I think it'd be good to add a test case showing what this PR fixes. It'll also help us catch regressions in the future. 🙇

done, i have added tests and all passed successfully

[erikian]

@dmazzella following the repro steps in #3934 and replacing node_modules/@electron-forge/plugin-vite/dist/VitePlugin.js in the repro project with packages/plugin/vite/dist/VitePlugin.js built on your branch does result in the app.asar.unpacked folder being created with the following structure:

app.asar.unpacked
└── node_modules
    └── better-sqlite3
        ├── bin
        │   └── darwin-arm64-140
        │       └── better-sqlite3.node
        └── build
            └── Release
                ├── better_sqlite3.node
                └── test_extension.node

However, I bisected this regression to #3583, specifically this code was handling the unpacking before it was removed: 5485969#diff-d1d8524b16d69b5d383151b395f4ac4151282b39dac16af4bd833cae8b689c8cL123-L126. If I restore that code, I get:

app.asar.unpacked
└── node_modules
    └── better-sqlite3
        └── build
            └── Release
                └── better_sqlite3.node

...which I think makes more sense since build/Release/better_sqlite3.node is the only binding used in runtime as far as I can tell. The previous approach is also faster than the one you're proposing (yarn package takes around 4s for me, versus ~18s in your branch).

Do you think it would be possible to bring your approach closer to what we had before #3583 in terms of performance and behavior? I'm not exactly sure why that code was removed (@caoxiemeihao / @erickzhao probably have the context here), but another option would be restoring it, as long as that doesn't cause any unwanted side effects.

[dmazzella]

I updated the PR to add flora-colossus (version 2.x which is CJS-compatible), which is undoubtedly the best approach in terms of functionality and performance.