Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] CLI-less webpack build #1098

Closed
Sayan751 opened this issue May 14, 2019 · 11 comments
Closed

[Proposal] CLI-less webpack build #1098

Sayan751 opened this issue May 14, 2019 · 11 comments

Comments

@Sayan751
Copy link
Member

I'm submitting a feature request (proposal)

  • Library Version:
    aurelia-cli v1.0.0-beta.15

Please tell us about your environment:

  • Operating System:
    Windows 10

  • Node Version:
    11.14.0

  • NPM Version:
    6.9.0
  • Browser:
    NA

  • Language:
    NA

  • Loader/bundler:
    Webpack

Current behavior:

The current build and run pipeline for webpack projects looks a bit overkill.

  • What is the expected behavior?

It can be made a lot easier if the layer offered by aurelia-cli tasks are removed, and the default webpack and webpack-dev-server CLI do their job.

  • What is the motivation / use case for changing the behavior?

The idea to have a aurelia-cli-less webpack build for a cli-genrated project was introduced to me here. It makes less clutter, gives more power to the developer, and lesser things to maintain for Aurelia team.

I have prepared the following "thing" to propose the solution (and mostly as a note to me to understand au build, and au run, as I am not a heavy user of aurelia-cli).


Solution proposal

TL;DR

No surprise that it can be done easily using npm scripts and webpack CLI parameters.

In order to have a cli-less webpack build, and run for the webpack users, it is first needed to evaluate what au-cli currently offers. At present au-cli offers 2 commands au build (utilizes webpack) and au run (utilizes webpack-dev-server). Following sections explore these 2.

au build

The webpack.config.js exports a function that creates the config based on the supplied arguments.

module.exports = ({ production, server, extractCss, coverage, analyze, karma } = {}) =>({
    // webpack config for magic
})

au build via buid.ts script collects the values for these arguments from CLI arguments, and aurelia.json and uses Webpack API to build the app. Hypothetically considering, if that is the only function of build.ts, it can be simply replaced with the following command line (refer webpack/webpack#2254 (comment)).

webpack --env.production --env.server --env.extractCss --env.coverage --env.analyze --env.karma
Reducing the number of arguments for webpack.config

From a very naive perspective, there are 3 different setups:

  • development,
  • production, and
  • tests.

From this perspective, the karma, and coverage parameters can be replaced with a single parameter tests.

I am not sure about the utility of the server parameter. To me it looks like it is used only during development as a metadata for HtmlWebpackPlugin. However, it is not used in index.ejs. Another usage of this is the conditional application of CopyWebpackPlugin during production or development build. This can also be replaced with the new !tests parameter.

The other two parameters extractCss, and analyze can be kept untouched.

With this, the resultant npm scripts might look something like this.

scripts:{
    ...
    "build": "webpack --env.production --env.extractCss",
    "build:dev": "webpack --env.extractCss",
    "analyze": "webpack --env.analyze",
    "test": "au test"
    ...
}
Removing other sources of configuration

The 3 CLI arguments utilized by the script are analyze, env, and watch, all of which are replaceable using simply the webpack CLI arguments (as shown above).

Another source of build parameters is from aurelia.json#build#options

"build": {
    "options": {
        "server": "dev",
        "extractCss": "prod",
        "coverage": false
    }
},

Based on this configuration, the build script decides whether to apply this configuration for current environment, that is production, or development. Again all these options can be de/activated using npm scripts. CLI can generate a recommended script for each of this environment. If the need be the app developer can always trivially change the parameters as per need.

The other tasks, such as deleting the previous distributions and configuring environment, can be left as it is as a gulp task, which is invoked as pre-task (either by pre hook or using npm-run-all).

au run

This is also very similar with au build, in terms of that it too collects the configurations from CLI, aurelia.json, and webpack.config.js and runs the app using webpack-dev-server with accumulated configuration options.

All the CLI options for the au run can be replaced with npm scripts as shown above. And the recommended defaults can directly be baked into the generated webpack.config.js.

Following code fragment is not really needed for HMR or reloading the app. I tried without both the options (for HMR I referred the webpack guide), and it worked as expected, both with or w/o HMR. I think these are some old artifacts.

  if (project.platform.hmr || CLIOptions.hasFlag('hmr')) {
    config.plugins.push(new webpack.HotModuleReplacementPlugin());
    config.entry.app.unshift('webpack-dev-server/client', 'webpack/hot/dev-server');
  } else {
    // removed "<script src="/webpack-dev-server.js"></script>" from index.ejs in favour of this method
    config.entry.app.unshift('webpack-dev-server/client');
  }

POC

For POC, please refer default-ts-webpack-app in this repo which uses the webpack and webpack-dev-server cli solely to build and run the cli-generated default TS-Webpack app. The README shows the modified scripts. For specific details refer Sayan751/aurelia-cli-projects@af008ef#diff-782ee31feceb525ffde151015edd994a.

Note1: My analysis here is solely based on the cli-generated default TS-Webpack app.

Note2: The user acceptance needs to be considered. If there is a majority of users accustomed with au flavored tasks, it might not be a welcoming change.

Lastly, sorry for the wall of text :)

@3cp
Copy link
Member

3cp commented May 15, 2019

@jods4 @fkleuver can you guys share your webpack setup here too?

@Sayan751 big thanks for your effort! This is a great start.

It's all about how aggressive we can be, I would vote to get rid of aurelia.json and start very clean :)
I don't think user acceptance is a big issue, if this is going to be only simpler.

@fkleuver
Copy link
Member

This (or some slight variation of it) is what I use in most of my projects:

const { AureliaPlugin } = require('aurelia-webpack-plugin');
const HtmlWebpackPlugin = require('html-webpack-plugin');
const path = require('path');

const root = path.resolve(__dirname);

const dist = path.join(root, 'dist');
const src = path.join(root, 'src');
const node_modules = path.join(root, 'node_modules');

const loadPostCssPlugins = function() {
  return [
    require('autoprefixer')({ browsers: ['last 2 versions'] })
  ];
};

const mimetype = {
  woff: 'application/font-woff',
  woff2: 'application/font-woff',
  tff: 'application/octet-stream',
  eot: 'application/vnd.ms-fontobject',
  svg: 'application/svg+xml',
};

const load = {
  style: {
    loader: 'style-loader',
  },
  css: {
    loader: 'css-loader',
  },
  postCss: {
    loader: 'postcss-loader',
    options: {
      plugins: loadPostCssPlugins,
    },
  },
  sass: {
    loader: 'sass-loader',
  },
  html: {
    loader: 'html-loader',
  },
  ts: {
    loader: 'ts-loader',
  },
  images: {
    loader: 'url-loader',
    options: {
      limit: 8192,
    },
  },
  woff: {
    loader: 'url-loader',
    options: {
      limit: 8192,
      mimetype: mimetype.woff,
    },
  },
  woff2: {
    loader: 'url-loader',
    options: {
      limit: 8192,
      mimetype: mimetype.woff2,
    },
  },
  tff: {
    loader: 'url-loader',
    options: {
      limit: 8192,
      mimetype: mimetype.tff,
    },
  },
  eot: {
    loader: 'url-loader',
    options: {
      limit: 8192,
      mimetype: mimetype.eot,
    },
  },
  svg: {
    loader: 'url-loader',
    options: {
      limit: 8192,
      mimetype: mimetype.svg,
    },
  },
  file: {
    loader: 'file-loader',
  },
};

module.exports = function configure(mode) {
  return {
    mode: mode || 'development',
    resolve: {
      extensions: ['.ts', '.js'],
      modules: [src, node_modules],
    },
    entry: {
      app: ['aurelia-bootstrapper'],
    },
    output: {
      path: dist,
      publicPath: '',
      filename: '[name].js',
      sourceMapFilename: '[name].map.js',
      chunkFilename: '[name].chunk.js',
    },
    watch: mode === 'development',
    devtool: 'source-map',
    devServer: {
      contentBase: dist,
      lazy: false,
      open: true,
      overlay: {
        warnings: true,
        errors: true,
      },
    },
    module: {
      rules: [
        {
          test: /\.scss$/i,
          issuer: [{ not: [{ test: /\.html$/i }] }],
          use: [load.style, load.postCss, load.sass],
        },
        {
          test: /\.css$/i,
          issuer: [{ not: [{ test: /\.html$/i }] }],
          use: [load.style, load.postCss],
        },
        {
          test: /\.scss$/i,
          issuer: [{ test: /\.html$/i }],
          use: [load.css, load.postCss, load.sass],
        },
        {
          test: /\.css$/i,
          issuer: [{ test: /\.html$/i }],
          use: [load.css, load.postCss],
        },
        {
          test: /\.html$/i,
          use: [load.html],
        },
        {
          test: /\.ts$/,
          use: [load.ts],
        },
        {
          test: /\.(png|gif|jpg|cur)$/i,
          use: [load.images],
        },
        {
          test: /\.woff2(\?v=[0-9]\.[0-9]\.[0-9])?$/i,
          use: [load.woff2],
        },
        {
          test: /\.woff(\?v=[0-9]\.[0-9]\.[0-9])?$/i,
          use: [load.woff],
        },
        {
          test: /\.(ttf|eot|svg|otf)(\?v=[0-9]\.[0-9]\.[0-9])?$/i,
          use: [load.file],
        },
      ],
    },
    plugins: [
      new AureliaPlugin(),
      new HtmlWebpackPlugin({
        template: 'index.ejs',
        metadata: {
          dev: mode !== 'production',
        },
      }),
    ],
  };
};

@3cp
Copy link
Member

3cp commented May 15, 2019

Here is @jods4's minimal webpack setup for reference https://github.com/jods4/aurelia-webpack-build

@Sayan751
Copy link
Member Author

I would like to point out that a webpack config that includes more loaders and plugins to cover wise range of use cases is useful for the developers just starting with webpack.

If the aim is to have a simpler/minimal webpack config, then the examples of some involved cases must be included in comments along with the use-cases.

@3cp
Copy link
Member

3cp commented May 15, 2019

Agree, those config was for reference, we sure need to retain all the supported features (less/scss/postcss/http1/2) in our webpack offering.

@Sayan751
Copy link
Member Author

I have started working on this. I had an issue with dealing the environment.ts file. The problem is as we don't have the CLI in place, setting the environment variable in a gulp task (for the prebuild activities) was a bit tricky and verbose.

I wrote a very basic loader that merges the customization from any_config.any_env.json to any_config.json when the active environment (option in the loader) in any_env. This is useful for merging production specific configurations, such as endpoints, api keys, etc. w/o necessarily duplicating those.

@3cp
Copy link
Member

3cp commented May 15, 2019

I am no webpack master. Here is a possible simple solution came to me.

Keep env config out of src folder, as environment/dev.js ... Don't copy the file, but use alias in webpack config.

resolve: {
  alias: {
    'environment': path.resolve(__dirname, 'environment', envName + '.js_or_ts')
  }
}

The down side is you have to do import ... from 'environment', cannot do import ... from './environment'.

@Sayan751
Copy link
Member Author

I agree that the config/environment files need to be kept outside src.

I considered the environment file as general application settings. In that context, certain settings are common among different environments (dev/prod), and certain stuff has environment specific value. With the merging-via-loader approach, a developer does not need to duplicate these values. With the alias approach, the duplication of config cannot be avoided.

As I saw the environment.ts file as configuration file, it was odd for me to see the configurations in a TS file as opposed to a JSON file. Is there any particular reason to keep the config in a JS/TS file? Or the way I see it, is not the way it was originally though out?

@3cp
Copy link
Member

3cp commented May 16, 2019

It's probably due to the original cli-bundler (amodro based) doesn't support tracing JSON files out of the box. The limitation doesn't exist in latest cli.

@Sayan751
Copy link
Member Author

Short update

I have prepared the loader, and documented it here: https://github.com/Sayan751/app-settings-loader, so that interested readers can check that and provide feedback if the concept has any shortcomings. I also needed similar loader for one of my other project. Therefore, I have put it in a different package altogether. I will continue with testing the changes for CLI.

@3cp
Copy link
Member

3cp commented May 19, 2019

Your loader looks intuitive to understand and use. It can also be used for any src/foo/bar.json with src/foo/bar.development.json for any env sensitive logic.

Sayan751 added a commit to Sayan751/cli that referenced this issue May 25, 2019
Usage of aurelia-cli is removed from building and running
the webpack apps. This gives developers more control over
webpack, and a way to easily change the configuration,
without aurelia-cli meddling in between.

The commands `au run` or `au build` will not work anymore
in the webpack apps, use `npm start`, or `npm run build`
instead. This commit closes the webpack part of the
the issue aurelia#1098.
Sayan751 added a commit to Sayan751/cli that referenced this issue Jul 13, 2019
Usage of aurelia-cli is removed from building and running
the webpack apps. This gives developers more control over
webpack, and a way to easily change the configuration,
without aurelia-cli meddling in between.

The commands `au run` or `au build` will not work anymore
in the webpack apps, use `npm start`, or `npm run build`
instead. This commit closes the webpack part of the
the issue aurelia#1098.

Conflicts:
	skeleton/webpack/webpack.config.js
@3cp 3cp closed this as completed Sep 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants