PR feedback 2

blueprints/ember-electron/index.js

@@ -7,7 +7,6 @@ const fs = require('fs');
 const readFile = promisify(fs.readFile);
 const writeFile = promisify(fs.writeFile);
 const YAWN = require('yawn-yaml/cjs');
-const SilentError = require('silent-error');
 const {
   upgradingUrl,
   routingAndAssetLoadingUrl,
@@ -25,40 +24,38 @@ module.exports = class EmberElectronBlueprint extends Blueprint {
     return entityName;
   }
 
-  beforeInstall() {
-    if (fs.existsSync(electronProjectPath)) {
-      return Promise.reject(
-        new SilentError([
-          `Cannot create electron-forge project at './${electronProjectPath}'`,
-          `because a file or directory already exists there. Please remove/rename`,
-          `it and run the blueprint again: 'ember generate ember-electron'.`
-        ].join(' '))
-      );
-    }
-
+  async afterInstall() {
     if (fs.existsSync('ember-electron')) {
       this.ui.writeLine(chalk.yellow([
         `\n'ember-electron' directory detected -- this looks like an ember-electron`,
         `v2 project. Setting up an updated project will not be destructive, but you`,
         `should read the upgrading documentation at ${upgradingUrl}.\n`
       ].join(' ')));
     }
-  }
 
-  async afterInstall() {
     await this.updateEnvironmentConfig();
     await this.updateTravisYml();
     await this.updateEslintIgnore();
     await this.updateEslintRc();
-    await this.createElectronProject();
+
+    if (!fs.existsSync(electronProjectPath)) {
+      await this.createElectronProject();
+    } else {
+      this.ui.writeLine(chalk.yellow([
+        `An electron-forge project already exists at './${electronProjectPath}'.`,
+        `If you're running the blueprint manually as part of an ember-electron`,
+        `upgrade, make sure to check for upgrade instructions relevant to your`,
+        `version upgrade at ${upgradingUrl}.\n`
+      ].join(' ')));
+    }
   }
 
   async updateEnvironmentConfig() {
     this.ui.writeLine(chalk.green('Updating config/environment.js'));
 
     let contents = (await readFile('config/environment.js')).toString();
 
-    const rootURLRegex = /(\srootURL:)/;
+    const rootURLRegex = /(\srootURL\s*:)/;
     if (rootURLRegex.test(contents)) {
       contents = contents.replace(rootURLRegex, `$1 process.env.EMBER_CLI_ELECTRON ? '' :`);
     } else {
@@ -70,7 +67,7 @@ module.exports = class EmberElectronBlueprint extends Blueprint {
       ].join(' ')));
     }
 
-    const locationTypeRegex = /(\slocationType:)/;
+    const locationTypeRegex = /(\slocationType\s*:)/;
     if (locationTypeRegex.test(contents)) {
       contents = contents.replace(locationTypeRegex, `$1 process.env.EMBER_CLI_ELECTRON ? 'hash' :`);
     } else {
@@ -82,15 +79,6 @@ module.exports = class EmberElectronBlueprint extends Blueprint {
       ].join(' ')));
     }
 
-    contents = [
-      contents,
-      ``,
-      `function getElectronRootURL() {`,
-      `  return process.env.EMBER_ENV === 'test' ? '../' : '';`,
-      `}`,
-      ``
-    ].join('\n');
-
     await writeFile('config/environment.js', contents);
   }
 

forge/files/src/handle-file-urls.js

@@ -1,10 +1,7 @@
 const { protocol } = require('electron');
 const { fileURLToPath } = require('url');
 const path = require('path');
-const fs = require('fs');
-const { promisify } = require('util');
-
-const access = promisify(fs.access);
+const { promises: { access } } = require('fs');
 
 //
 // Patch asset loading -- Ember apps use absolute paths to reference their
@@ -26,4 +23,4 @@ module.exports = function handleFileURLs(emberAppDir) {
       callback(urlPath);
     }
   });
-}
+};

forge/files/src/index.js

@@ -31,7 +31,7 @@ app.on('ready', async () => {
     try {
       require('devtron').install();
     } catch (err) {
-      console.log('Failed to install Devtrom: ', err);
+      console.log('Failed to install Devtron: ', err);
     }
     try {
       await installExtension(EMBER_INSPECTOR);

forge/files/tests/index.js

@@ -10,12 +10,12 @@ app.on('ready', async function onReady() {
   try {
     require('devtron').install();
   } catch (err) {
-    console.log('Failed to install Devtrom: ', err);
+    console.log('Failed to install Devtron: ', err);
   }
   try {
     await installExtension(EMBER_INSPECTOR);
   } catch (err) {
-    console.log('Failed to install Ember Inspector: ', err)
+    console.log('Failed to install Ember Inspector: ', err);
   }
 
   await handleFileUrls(emberAppDir);

node-tests/unit/blueprint-test.js

@@ -38,14 +38,15 @@ describe('blueprint', function() {
     })
 
     async function getEnv() {
-      let environmentJs = path.join(__dirname, '..', 'fixtures', 'config-environment', 'environment.js');
+      let environmentJsFixture = path.join(__dirname, '..', 'fixtures', 'config-environment', 'environment.js');
+      let environmentJs = path.join(process.cwd(), 'config', 'environment.js');
 
       mkdirSync('config');
-      copyFileSync(environmentJs, path.join('config', 'environment.js'));
+      copyFileSync(environmentJsFixture, environmentJs);
 
       await blueprint.updateEnvironmentConfig();
 
-      let factory = require(path.join(process.cwd(), 'config', 'environment.js'));
+      let factory = require(environmentJs);
       return factory();
     }
 

tests/dummy/app/templates/docs/faq/routing-and-asset-loading.md

@@ -16,7 +16,7 @@ and we deploy it to `http://my-app.com`. This means that the URL `http://my-app.
 
 When running an Electron app, we serve the files out of the filesystem using a `file:` URL, which doesn't give us the flexibility of a web server -- the URL path cannot be a logical location, but must be a filesystem path that points directly to `index.html`. For example, when running a development build, the path might be `file:///Users/me/projects/my-app/electron-app/ember-dist/index.html`. We can't use the URL path to specify the Ember route because telling the browser to load `file:///Users/me/projects/my-app/electron-app/ember-dist/index.html/my-route` will not work -- it will look for a file named `my-route` inside a directory named `index.html`, and won't find it.
 
-So we have to go back to an older method of specifying logical locations, which is to use the URL hash. Fortunately, Ember supports this via the [HashLocation](https://api.emberjs.com/ember/release/classes/HashLocation) since that was the only option in older browsers. Since URL hashes don't affect how `file:` URLs are resolved to paths, we can tell the browser to load `file:///Users/me/projects/my-app/electron-app/ember-dist/index.html#my-route`, and it will load the `index.html` and then Ember can resolve the route using the path contained in the URL hash.
+So we have to go back to an older method of specifying logical locations, which is to use the URL hash. Fortunately, Ember supports this via the [HashLocation](https://api.emberjs.com/ember/release/classes/HashLocation) since that was the only option in older browsers. Since URL hashes don't affect how `file:` URLs are resolved to paths, we can tell the browser to load `file:///Users/me/projects/my-app/electron-app/ember-dist/index.html#/my-route`, and it will load the `index.html` and then Ember can resolve the route using the path contained in the URL hash.
 
 This is why `ember-electron`'s blueprint configures the application's `config/environment.js` with the following entry:
 

tests/dummy/app/templates/docs/guides/upgrading.md

@@ -75,7 +75,7 @@ Either way, you will need to update the configuration since the format has chang
 
 ### main.js
 
-The equivalent of `ember-electron/main.js` is `electron-app/src/index.js`, so you'll want to put the contents of your `main.js` there (note that `src/index.js` is specified by the `main` entry in `electron-app/package.json`, so you can name it whatever/put it wherever you want as long as you update the `main` entry accordingy). Because the Electron project structure differs a little from `ember-electron` 2.x's, you'll need to replace anywhere you reference the path to the Ember application directory (`../ember`) with the new path (`../ember-dist`).
+The equivalent of `ember-electron/main.js` is `electron-app/src/index.js`, so you'll want to put the contents of your `main.js` there (note that `src/index.js` is specified by the `main` entry in `electron-app/package.json`, so you can name it whatever/put it wherever you want as long as you update the `main` entry accordingly). Because the Electron project structure differs a little from `ember-electron` 2.x's, you'll need to replace anywhere you reference the path to the Ember application directory (`../ember`) with the new path (`../ember-dist`).
 
 ### test-main.js
 
@@ -112,13 +112,81 @@ Anything else that was in the `ember-electron` in 2.x should be "just files" --
 
 Since 2.x, we have removed `electron-protocol-serve` from the default blueprint in favor of loading the Ember app using `file:` URLs. The instructions above should get you set up to run properly, but if you app has any references/assumptions around the URL used to load the Ember app, you'll need to update them. If you added `webSecurity: false` to work around issues caused by `electron-protocol-serve` (as described [here](/versions/v2.10.2/docs/faq/common-issues#what-is-electron-protocol-serve-and-why-do-i-need-this-)) you should be able to remove it now.
 
-You can read more about the removal of `electron-protocol-serve` and loading from `file:` URLs [here](../faq/routing-and-asset-loading).
+Another effect of switching from `serve:` URLs to `file:` URLs is that you may need to migrate data stored in browser storage such as `localStorage` or `IndexedDB` or your users could experience data loss. If a user has been running a version of your application that uses `serve:` URLs, then the browser will have any such data associated with the `serve://dist` domain, and the browser's security measures to prevent one site from accessing another site's will prevent your app, which accessed via a `file:` URL, from accessing the previously-created data.
+
+Unfortunately, Electron doesn't currently provide any mechanisms to address this, so if your application has stored any such data on users' systems, and it's critical that it remain intact when the user updates to a version of the application that uses `file:` URLs, you'll have to migrate it. Here's an example of how you might do it for `localStorage`:
+
+```javascript
+// src/index.js
+const { protocol, BrowserWindow } = require('electron');
+const { promises: { writeFile } } = require('fs');
+const { fileSync } = require('tmp');
+
+function needsMigration() {
+  // You'll want some way of determining if the migration has already happened,
+  // the when the app starts it doesn't always re-copy the data from the
+  // `serve://dist`-scoped, overwriting any changes the user has since made to
+  // the `file:`-scoped data. For example, you might use `electron-store` to
+  // keep track of whether you've run the migration or not.  
+}
+
+if (needsMigration()) {
+  // Register the `serve:` scheme as privileged, like `electron-protocol-serve`
+  // does. This enables access to browser storage from pages loaded via the
+  // `serve:` protocol. This needs to be done before the app's `ready` event.
+  protocol.registerSchemesAsPrivileged([
+    {
+      scheme: 'serve',
+      privileges: {
+        secure: true,
+        standard: true
+      }
+    }
+  ]);
+
+  app.on('ready', async () => {
+    // Set up a protocol handler to return empty HTML from any request to
+    // `serve:` URLs, so we can load `serve://dist` in a browser window and use
+    // it to access localStorage
+    protocol.registerStringProtocol('serve', (request, callback) => {
+      callback({ mimeType: 'text/html', data: '<html></html>' });
+    });
+
+    // Navigate to our empty page in a hidden browser window
+    let window = new BrowserWindow({ show: false });
+    try {
+      await window.loadURL('serve://dist');
+
+      // Get a JSON-stringified version of this origin's entire localStorage
+      let localStorageJson = await window.webContents.executeJavaScript('JSON.stringify(window.localStorage)');
+
+      // Create an empty HTML file in a temporary location that we can load via a
+      // `file:` URL so we can write our values to the `file:`-scoped localStorage.
+      // We don't do this with a protocol handler because we don't want to mess
+      // with how `file:` URLs are handled, as it could cause problems when we
+      // actually load Ember app over a `file:` URL.
+      let tempFile = fileSync();
+      await writeFile(tempFile.name, '<html></html>');
+      await window.loadFile(tempFile.name);
+
+      // Iterate over the values and set them in file:'s localStorage
+      for (let [ key, value ] of Object.entries(JSON.parse(localStorageJson))) {
+        await window.webContents.executeJavaScript(`window.localStorage.setItem('${key}', '${value}')`);
+      }
+    } finally {
+      window.destroy();
+    }
+  });
+}
+```
+
+This would be somewhat more complicated for storages with more structure/data formats like `IndexedDB`, but this should serve as a template for how the data could be migrated.
 
 # Upgrading from 3.0.0-beta.2
 
 Between `3.0.0-beta.2` and `3.0.0-beta.3` we removed `electron-protocol-serve` from the default blueprint as explained [here](#electron-protocol-serve). The best way to upgrade from a `3.0.0` beta version before `3.0.0-beta.3` is to:
 
-1. Start with a clean working directory
+1. Start with a clean working tree (no uncommitted changes)
 2. Update `ember-electron` to the latest version
 3. Rerun the blueprint (`ember g ember-electron`) overwriting all files when prompted
 4. Look at the git diff and re-introduce any changes/customizations you previously made to the affected files
@@ -131,4 +199,6 @@ The changes you should end up with are:
 * Changes to `electron-app/src/index.js` and `electron-app/tests/index.js` to switch from `electron-protocol-serve` to `file:` URLs
 * Removal of `electron-protocol-serve` from `electron-app/package.json`
 
+If your application uses any browser storage like `localStorage` or `IndexedDB`, you may need to migrate the data so it's accessible from `file:` URLs -- make sure to read the [this](#electron-protocol-serve) section for more info.
+
 You can read more about the removal of `electron-protocol-serve` and loading from `file:` URLs [here](../faq/routing-and-asset-loading).