search

electron-userland / electron-spellchecker

Implement spellchecking, correctly
MIT License
238 stars 83 forks source link

Use of spawn-rx causes WACK "Package Sanity" test failures #87

Closed DanielRHarris closed 7 years ago

DanielRHarris commented 7 years ago

Hi,

This is a continuation of https://github.com/electron-userland/electron-builder/issues/2029.

The spawn-rx module used in this package makes all sorts of calls to CMD, BASH, etc, which cause the failure of the "Package Sanity" portion of the WACK test. With the release of Windows S, all Desktop Bridge applications are required to pass the complete WACK test, which is impossible if electron-spellchecker is included.

MarshallOfSound commented 7 years ago

@DanielRHarris The spawn-rx usage in this repo afaics (from a quick github code search) is limited to only the linux platform.

See: https://github.com/electron-userland/electron-spellchecker/blob/6c61f8b156b22d1f40f0869c545f02d26466d876/src/spell-check-handler.js#L590

Are you sure that it's because of this module?

DanielRHarris commented 7 years ago

@MarshallOfSound That's the kicker -- spawn-rx is only being used on linux platforms, but because it's being included with electron-spellchecker, it ends up in the final Windows build as well, thus causing the issues.

Simply removing spawn-rx from node_modules, and the spawn-rx import and "if (process.platform === 'linux') {}" segment in spell-check-handler solves the issue.

A replacement for spawn-rx is probably necessary... I wonder if child_process.spawn can be used instead?

MarshallOfSound commented 7 years ago

@DanielRHarris We might have to reach out to microsoft to get confirmation on this but my understanding of this is that if the WACK test now includes checks for Windows 10 S (and these checks are enforced) then no Electron app will work as I think node / electron uses API's that would not pass those tests. I'm updating my WACK right now to investigate 👍

DanielRHarris commented 7 years ago

As am I -- my previous post is my best guess, should know whether it's really accurate in a few minutes.

MarshallOfSound commented 7 years ago

@DanielRHarris Looks like my WACK passed, even though the Package Sanity tests failed, they are marked as optional in the WACK report. Bearing in mind I've released an update to my Electron app on the windows store in the last few days I don't think this is blocking UWP releases of Electron apps

MarshallOfSound commented 7 years ago

From the report:

The tests in this section are informational only and will not be used to evaluate your app during Windows Store onboarding. Investigating failures is recommended to ensure that users will not be impacted.

DanielRHarris commented 7 years ago

Unfortunately my guess proved to be innacurate. I'm still getting the sanity test problems, making it likely that it's a core electron issue as you thought, and not an issue with electron-spellchecker.