Windows fixes #12
Windows fixes #12
Conversation
This reverts commit 9f35963.
| @@ -0,0 +1,2 @@ | |||
| const getFileIcon = process.platform === 'darwin' ? require('./mac') : null | |||
There was a problem hiding this comment.
It would be better to export empty function, instead of null to avoid ... is not a function errors
| // Core plugins | ||
| export { default as apps } from './apps' | ||
| export { default as autocomplete } from './autocomplete' | ||
| export { default as contacts } from './contacts' | ||
| export let contacts |
There was a problem hiding this comment.
I'd just rewrite this block to module.exports instead of static export. It will allow exporting some plugins conditionally, instead of exporting undefined and add conditions above.
|
@staff0rd can you please review my commit and check that it still works on windows? Let me know and I'll merge this PR. Thanks for contribution! |
|
Your changes look good - note that I am absent of any electron or react experience, and my ES6 is based mostly on Typescript, so I am only guessing at conventions in this code. I'll test this tomorrow, been on the beers tonight :) |
|
I tested feature/move-osx-plugins on a windows machine - works great. |
|
@staff0rd awesome, thanks for your time! |
This ends up with the app running on windows by including the nodobjc dependency only when building on mac, and explicitly excluding the contacts & define plugins. The apps plugin is implicitly excluded due to the try/catch on plugins (windows will throw on the path '/Applications').
The explicit exclusions are a bit hacky - the plugins should probably instead define which platform they run on in config and load only those specific to the currently detected platform.
getFileIcon is setup such that other platform's implementations could be added at a later date.
Tested on both mac & windows. (Mac retains full functionality).