Add option to set stable or insiders build#20
Conversation
Addresses microsoft#17 Adds an extension options page, stores the value in sync storage, and switches the domain based on the synced value By default, it uses the stable build, since I imagine what most users would prefer
|
|
||
| let preferredVSCodeBuildDomain; | ||
|
|
||
| chrome.storage.sync.get({ |
There was a problem hiding this comment.
Because this is a callback, could it be possible that preferredVSCodeBuildDomain is undefined or stale at the time that a redirect is invoked? Could we instead read this value at the time of invoking a redirect?
There was a problem hiding this comment.
If the sync get fails (the value isn't found), it's support to use the default value passed in to it. So I don't know what case it wouldn't be set, but I'm not an expert with this API so that could be a problem.
I did consider reading the value each time, but was worried that might cause some delay, since it's an extra I/O check per invocation. But, it's also a small enough value being read that I don't think it'd matter very much.
I'll change this to check per invocation and see if I notice any extra latency. If not, I'll leave that change. If I do notice latency, I'll set preferredVSCodeBuildDomain to the default so it will always work, even if that value is old
There was a problem hiding this comment.
I tired playing with it a little, but unfortunately since this is a network call, it's asynchronous and doesn't seem to play nice with the rest the code. I tried async-ifying some of the code to see if that would help (though I am not well versed with JS's async flow), and that didn't seem to get things to work.
I've updated it to set a default value, so we should always have something valid. Is that satisfactory or would you like me to try something else?
This will ensure that we always have a value, even if it's not the chosen one
This also changes the icon size to be 128x128, as I found larger ones threw errors
|
Also added an insider icon, so the tool bar button will reflect the settings |
Addresses #17
Adds an extension options page, stores the value in sync storage, and switches the domain based on the synced value
By default, it uses the stable build, since I imagine what most users would prefer