preact-cli refactor: Separate internal concepts of config and env

refactor: Separate internal concepts of config and env

Open rschristian opened this issue 3 years ago • 1 comments

What kind of change does this PR introduce?

refactor

Did you add tests for your changes?

Existing cover

Summary

In CLI, we mix the concepts of 'configuration' and 'environment' quite heavily; this means mixing together properties like isProd and pkg.dependencies. Accordingly, in different parts of the source, we refer to this mega variable as env or config.

What this PR proposes is to separate these out: app configuration values are separate to environment values.

This came about from a user messaging me about checking env in their preact.config.js and seeing all sorts of values that would never be of any use to them. See below for what console.log(env) results in.

Full `env` object

{
  _: [],
  prerender: false,
  sw: false,
  src: '/home/ryun/Projects/cli/src',
  dest: '/home/ryun/Projects/cli/build',
  cwd: '/home/ryun/Projects/cli',
  babelConfig: '.babelrc',
  json: undefined,
  template: undefined,
  preload: false,
  analyze: undefined,
  prerenderUrls: 'prerender-urls.json',
  brotli: false,
  'inline-css': true,
  config: 'preact.config.js',
  c: 'preact.config.js',
  production: true,
  isProd: true,
  isWatch: false,
  source: [Function (anonymous)],
  manifest: {
    name: 'foo',
    short_name: 'foo',
    start_url: '/',
    display: 'standalone',
    orientation: 'portrait',
    background_color: '#fff',
    theme_color: '#673ab8',
    icons: [ [Object], [Object] ]
  },
  pkg: {
    private: true,
    scripts: {
      build: 'preact build --no-prerender --no-sw',
      'serve:prod': 'sirv build --port 3000 --cors --single',
      'serve:dev': 'preact watch',
      lint: 'eslint src',
      test: 'jest'
    },
    devDependencies: {
      'preact-cli': 'file:.yalc/preact-cli',
      prettier: '^2.7.1',
      'prettier-config-rschristian': '^0.1.1',
      'sirv-cli': '1.0.3'
    },
    dependencies: {
      preact: '^10.3.2',
      'preact-iso': '^2.3.0',
      'preact-render-to-string': '^5.1.4',
      'preact-router': '^3.2.1'
    }
  },
  isServer: false,
  dev: false,
  ssr: false
}

While we could just limit what we return to users, this also creates potential issues in our source. It's difficult to know when and where we set what values as this is treated like a global, just a less convenient one. For example, I went on a bit of a hunt earlier in trying to find out where in the world we set isServer. Hint: it's absolutely no where near where we set isProd or the like.

Additionally, there's an issue of multiple semi-duplicated values: production and isProd, ssr and isServer. While of these are set together as aliases, others are not, and there's always the potential issue of them not being in sync. We should cut these down into a consistent list of idiomatic conditions for both us and users to use.

Does this PR introduce a breaking change?

Yes, some values from env will be stripped. See changeset for better log.