Fix: Webpack fallback release config

[codebykat]

Fix

Alternative to #3231, trying to see if this fixes the build artifacts.

Ref: https://medium.com/@6465660/webpack-compilation-error-critical-dependency-the-request-of-a-dependency-is-an-expression-13350139f2a5

And I'm mad about it 😠

[twstokes]

✅ I was able to successfully run and log in (via WordPress.com) using the Mac artifact from this job.

[codebykat]

I also downloaded the Mac artifact and was able to log in. Can we find someone to test these on the Linux and Windows VMs?

Assuming they work, @roundhill how do you feel about proceeding with this fix?

[twstokes]

Can we find someone to test these on the Linux and Windows VMs?

I can be that someone, with real machines. 👍

[twstokes]

@codebykat is starting the app and logging in enough to test for this change?

[codebykat]

Yes! If you're able to log in that definitely shows it has the correct config.

[twstokes]

Windows 64-bit installer: ✅
Debian 64-bit: ⚠️ Application loaded and the WordPress.com auth flow worked up until Simplenote was brought back to the foreground and it showed an auth error. Force-quitting and reloading seemed to resolve it.

[roundhill]

Here's an approach for loading the config for webback from the file system:

function getConfigFromFileSystem() {
  const configPath = fs.existsSync('./config-local.json')
    ? './config-local.json'
    : './config.json';

  return fs.readFileSync(configPath, 'utf8');
}

module.exports = () => {
  const isDevMode = process.env.NODE_ENV === 'development';
  const config = JSON.parse(getConfigFromFileSystem());
  ...

This fixed the issue where getConfig() was being called before we set the fallback.

[codebykat]

Ugh. I've updated this PR to have webpack require fs and look to see which of the config files exist so that it can fall back without throwing a warning. I spent a VERY long time trying to allow get-config.js to require fs, but absolutely could not get webpack to provide it in the bundle no matter what I tried. As a result, the function getConfig is no more, and all uses of it now need to rely on the global config which is being magically provided by webpack. I made some config adjustments so TS is aware of the situation and won't complain.

I included a package upgrade as I got a notification about a high risk vulnerability in electron-updater, allowing a code signing bypass for Windows, which seemed important to address.

Once the artifacts build and we have tested them on all platforms, I think we've got our next (and hopefully final??) release candidate.

[roundhill]

Overall working well, I saw a weird thing with the webpack target for a while...

[roundhill]

Our config woes seem resolved 😅

• Build works with and without config-local.json present
• Sign-in works on both web and electron builds

[mokagio]

What happens here?

With my limited Electron + Webpack + TS understanding, I think at this point config would be undefined, making the if below unreachable. Unless... config is somehow injected?

[roundhill]

It's because config is actually set as a Webpack plugin, defined here:

simplenote-electron/webpack.config.js

Line 157 in 13cb5d1

config: JSON.stringify(config),

With my limited knowledge that is why it exists everywhere :)

[codebykat]

Yep exactly, as long as you don't define it inside the file, webpack will inject it. (If it IS defined in the file then webpack will assume you want a locally-defined version instead.)

[mokagio]

Thanks for the info @roundhill @codebykat

[mokagio]

@codebykat @roundhill I update the target branch for this PR to release/2.22.1, #3233, as discussed internally. TL;DR given 2.22.0 has already been tagged, any fix on top of it needs to come in a new version, in this case, a patch version or hotfix.