fix(app-extension): Allow the extension developer to specify the extension install flags, fix: #6352

[ejez]

What kind of change does this PR introduce? (check at least one)

• Bugfix
• Feature
• Documentation
• Code style update
• Refactor
• Build-related changes
• Other, please describe:

Does this PR introduce a breaking change? (check one)

• Yes
• No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

• It's submitted to the dev branch and not the master branch
• When resolving a specific issue, it's referenced in the PR's title (e.g. fix: #xxx[,#xxx], where "xxx" is the issue number)
• It's been tested on a Cordova (iOS, Android) app
• It's been tested on a Electron app
• Any necessary documentation has been added or updated in the docs (for faster update click on "Suggest an edit on GitHub" at bottom of page) or explained in the PR's description.

If adding a new feature, the PR's description includes:

• A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

An App Extension might include code that needs to be executed at the app runtime (ex: boot files provided by the ext), in this case the AE should be added to the app dependencies not devDependencies, if not, the ssr code resulting from quasar build -m ssr when used outside the app directory will not include the AE when it is npm/yarn installed. (see reproduction steps below)

When installing an App extension, quasar currently hardcodes the install flags to --save-dev for npm and --dev for yarn.

The PR allows the extension developer to decide where his ext should go, based on the code included in it. He can do so by adding the following to the ext package.json file:

  "quasar": {
    "extension": {
      "npmInstallFlags": "--save-prod",
      "yarnAddFlags": ""
    }
  }

If the above is not provided, then --save-dev (or --dev) will be used as default to keep the current behavior.

When the PR is accepted/finalized relevant docs will be added.

Note: I wanted to start a new project that will avail quasar docker images, but got into this constraining issue, hence this PR

See #6352

Issue reproduction steps

quasar create testapp
cd testapp
quasar ext add @m8a/apollo
quasar build -m ssr

output:

...
Tip: The output folder must be yarn/npm installed before using it,
      except when it is run inside your already yarn/npm installed project folder.

Now we want to copy and serve the built code from our web server (might be a different machine, but for this example we'll use another directory in the same machine)

cp -r dist/ssr /path/to/served/app
cd /path/to/served/app
yarn
yarn start

Now we get 500 | Internal Server Error, due missing extension code

@rstoenescu @hawkeye64 @thr0n

[hawkeye64]

@ejez Enlighten me, because maybe I am missing something. How is the Quasar extension handler supposed to read the package.json before it's installed?

[hawkeye64]

right, npm view first to get package.json info

[hawkeye64]

At work, after we compile everything, it's copied to a new location, without node_modules and then we install with --production, which does not install anything from devDeps

[ejez]

Issue updated with minimal reproduction steps of the problem

[ejez]

To quickly test the code introduced by this PR, do the same as the reproduction steps described, and in addition, copy the modified Extension.js , and use ext @ejez/apollo which has the key quasar.extension in package.json

quasar create ext-test-app
cd ext-test-app
curl -o node_modules/@quasar/app/lib/app-extension/Extension.js https://raw.githubusercontent.com/ejez/quasar/dev/app/lib/app-extension/Extension.js
quasar ext add @ejez/apollo
quasar build -m ssr
cp -r dist/ssr /path/to/served/app
cd /path/to/served/app
yarn
yarn start

Now the served app should work fine

[ejez]

You can also use the following to test different possibilities of the install flags:

ext-test.js

class Extension {
  // extracts extension params defined in its package.json file
  params () {
    try {
      // reads package.json key: 'quasar.extension' if defined.
      // execSync returns a buffer
      const paramsBuffer = require('child_process').execSync(
        `npm view --json @ejez/quasar-app-extension-apollo quasar.extension`
      )

      // parse the buffer and return an object
      return JSON.parse(paramsBuffer)
    } catch (err) {
      // when the extension is not published yet to npm, the above will result
      // in an error.
      return {}
    }
  }

  getInstallFlags (params) {
    // if install flags are not defined, we target devDependencies as default
    const npmInstallFlags = 'npmInstallFlags' in params
      ? params.npmInstallFlags
      : '--save-dev'
    const yarnAddFlags = 'yarnAddFlags' in params
      ? params.yarnAddFlags
      : '--dev'

    return {
      npm: npmInstallFlags,
      yarn: yarnAddFlags
    }
  }

  __installPackage (nodePackager, params) {
    const flags = this.getInstallFlags(params)
    // if flags are specified as empty strings, we don't concatenate
    const cmdParam = nodePackager === 'npm'
      ? flags.npm === '' ? ['install'] : ['install'].concat(flags.npm)
      : flags.yarn === '' ? ['add'] : ['add'].concat(flags.yarn)

    console.log(`${nodePackager} installation params are: ${cmdParam}`)
  }

  __uninstallPackage (nodePackager, params) {
    const flags = this.getInstallFlags(params)
    // if flags are specified as empty strings, we don't concatenate
    const cmdParam = nodePackager === 'npm'
      ? flags.npm === '' ? ['uninstall'] : ['uninstall'].concat(flags.npm)
      : flags.yarn === '' ? ['remove'] : ['remove'].concat(flags.yarn)

    console.log(`${nodePackager} removal params are: ${cmdParam}`)
  }
}

const ext = new Extension()

const p = [
  ext.params(),
  {},
  { npmInstallFlags: '--save-prod', yarnAddFlags: '' },
  { npmInstallFlags: '--save-dev', yarnAddFlags: '--dev' }
]

p.forEach(params => {
  console.log(params)
  ext.__installPackage('npm', params)
  ext.__uninstallPackage('npm', params)
  ext.__installPackage('yarn', params)
  ext.__uninstallPackage('yarn', params)
  console.log('******************')
})

node ext-test

[ejez]

Few clarifications about the added code:

npm view --json :

• throws error if the extension does not exist in npm registry, params() then returns an empty object (catch statement)

JSON.parse():

• JSON.parse(something) is a shorthand of JSON.parse(something.toString()) , toString gets called implicitly when using JSON.parse
  Quote from its spec:

  Let JText be ToString(text).

• if ext exists but key quasar.extension does not exist, execSync returns an empty buffer, which leads to JSON.parse('') (parsing empty string) which throws an error (catch statement covers this case)

[ejez]

In the last commit we use the same install flags for removal also, this is mainly to account for when the user has modified the default npm config (for ex if he has done npm config set save-dev=true)

[hawkeye64]

This is looking good. I've wanted this for quite some time, especially for UI-bases app-extensions. At work, we build all of our micro-services and client, them move it all sans node_modules, then run the install again with --production which only installs dependencies. For app-exts, we have to manually move them from devDependencies to dependencies. A more automated approach is appealing. 👍

[smolinari]

I'm not enough of a subject expert to offer anything to this that might improve it. Good job nonetheless!

Scott