feat(webpack):enable CLI-less webpack build + run #1137

Sayan751 · 2019-07-13T07:55:55Z

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 #1098.

Summary of changes:

Parameters consolidations and other minor changes in webpack.config.js. Removed the limiting options from ts-loader as it prevents compiling *.spec.ts files during test with karma(+webpack).
Replaced the au build and run scripts with npm scripts. Affected areas: package.json, aurelia_project/tasks, and release check tests. For pre-build cleanup activity, there is a small gulpfile introduced.
Changed tsconfig.json to have typeroots applied for all cases, without that the release checks were failing for couple of suites. Other changes here enable loading a json file with ES6 import syntax.
Lastly converted the environment.tss to json files, which are loaded using the app-settings-loader.

Note: This PR is an updated version of PR #1105 (justification)

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

jwx · 2019-07-13T11:18:08Z

While removing the dependency to the CLI for building and running the webpack apps is great, I think it should still be possible to use the au commands. Not necessary, but possible.

skeleton/webpack/gulpFile.js

skeleton/scaffold-navigation/src/main.ext

skeleton/webpack/webpack.config.js

- Gulp is removed from webpack project; del is replaced by clean-webpack-plugin - Environment import is retained for cli-buindler

3cp · 2019-07-17T04:05:13Z

I played your branch, the new app still has some gulp files in aurelia_project/tasks.

environment task can be removed because it's not used.
jest task can be removed. Plain jest command can replace it.
karma/protractor/cypress task I am not very sure.

aurelia_project/environments/* can be removed for webpack app.

Sayan751 · 2019-07-17T04:35:29Z

@3cp I have missed the tasks files in aurelia_project/tasks. I will add gulp back to the dependency of webpack projects. I will also remove the environment task. Not sure what this new-thing task do. Can you shed some light? For the test tasks I think it is better to handle that in another PR.

3cp · 2019-07-17T04:49:53Z

Are you talking about au generate value-converter ...? It's handled within aurelia-cli (with dep on gulp), plus those template files in app's local aurelia_project/generators/*. That part should be fine.

3cp · 2019-07-17T04:53:23Z

It's unfortunate this change could not get rid of all of aurelia-cli and gulp. 😄
Too many breaking changes if we do so.
Moving webpack build out of gulp will still please users.

The dependency to gulp is added back for the webpack projects as there are still some au tasks that depends on gulp. Additionally, removed environment task.

3cp · 2019-07-18T06:13:30Z

Since gulp is still there, I think we should have a run.ext doing something like this :-)

import NPM from 'aurelia-cli/lib/package-managers/npm';

export default function() {
  // first print out a warning to say `au run` is replaced by `npm start`
  return (new NPM()).install([], process.cwd(), 'start');
  // gulp understand the returned promise
}

For build.ext, it is return (new NPM()).install(['build'], process.cwd(), 'run');.

3cp · 2019-07-18T06:18:20Z

Oh no, just came into my mind, this PR will complicate #1132 where @shahabganji tries to add --host to au run.
Probably need to handle that through process.env?

Sayan751 · 2019-07-18T16:20:06Z

@3cp @shahabganji I have not went thoroughly with the other PR. However, if devs want to change any the default config, this PR will make it easier to manipulate any configuration option in webpack. This PR generates the webpack skeletons with following npm script.

"start": "webpack-dev-server --env.extractCss --port 8080"

Devs can now simply use npm start -- --host 127.0.0.1 --open, and that's it.

Sayan751 · 2019-07-18T16:43:10Z

Since gulp is still there, I think we should have a run.ext doing something like this :-)
import NPM from 'aurelia-cli/lib/package-managers/npm';

export default function() {
  // first print out a warning to say `au run` is replaced by `npm start`
  return (new NPM()).install([], process.cwd(), 'start');
  // gulp understand the returned promise
}
For build.ext, it is return (new NPM()).install(['build'], process.cwd(), 'run');.

@3cp I would suggest to have the following deprecation notice.

au run task is deprecated for the webpack projects, and a potential candidate for removal in future. Consider using npm start instead.

Otherwise, release checks needs to be written for au run, and au build which are otherwise removed.

jwx · 2019-07-18T19:23:22Z

I strongly disagree with removing the au commands for any CLI build. They might not do anything but run npm scripts, but they should still always be available. @Sayan751 @3cp

Sayan751 · 2019-07-18T19:37:13Z

@jwx That can be arranged as @3cp pointed out. In that case, the warning/deprecation notice can also be avoided. However, if those tasks are generated in the skeletons, there needs to be release check tests in place to assert the functionality.

One possibility is that in those tests it is asserted that au run executes npm start, and that's all. Every other tests that ran for npm start need not to be duplicated for au run. However, I am not sure how to make that assertion, maybe @3cp can shed some light. Other possibility is to simply replicate the tests for au run.

However, I would like to point out that devs can always write those tasks. The readme can be updated with copy-paste-able snippets.

3cp · 2019-07-19T04:36:39Z

We didn't assert in release-check, it's all predefined based on what features user selected (webpack or cli-bundler).

3cp · 2019-07-19T04:38:47Z

If we got the compatible run/build tasks, We might be able to avoid changing release-check tasks, less code change in this PR.

shahabganji · 2019-07-19T07:55:54Z

Devs can now simply use npm start -- --host 127.0.0.1 --open, and that's it.

@Sayan751, @3cp

What is more important than overriding port and host is the ability to run both the application and tests, in particular e2e test, in one terminal rather than separately running each one, I mean before that PR, we had to run the application first and then in another terminal run e2e tests either in interactive mode or headless; While in the case of CI automation, we need to run the application automatically in the same process(Kinda) as the e2e tests, and tear down everything whenever necessary. Some of the work which has been done in the above-mentioned PR is about that. checkout the --start argument, that will run the application, runs the e2e tests afterward and then tears down everything.

e.g. au cypress --run --start.

check out this line, in which we run the application before running cypress, the same scenario exists for protractor. Is there a way to replace that line of code if we use this PR? Or is there any alternative approaches?

Sayan751 · 2019-07-19T17:41:07Z

@shahabganji Does this support your use case?
https://github.com/bahmutov/start-server-and-test

Sayan751 · 2019-07-19T20:20:18Z

@3cp Does supporting the au tasks means supporting the same set of arguments to these tasks as well? Does that not make everything more complex than before?

shahabganji · 2019-07-20T04:27:55Z

@Sayan751

@shahabganji Does this support your use case?
https://github.com/bahmutov/start-server-and-test

I'll check it, but off the top of my head, it only supports cypress, correct me if I'm wrong; but CLI offers a bunch of combinations that need to be supported and have the same behavior, IMO.

Protractor + Built-in CLI-Bundler
Protractor + Webpack
Cypress + Built-in CLI-Bundler
Cypress + Webpack

This PR is about when we are using webpack, right? What about built-in cli bundler?

Sayan751 · 2019-07-20T12:57:04Z

@shahabganji I think I the package has nothing specific for cypress. In fact, I tested it on one of the generated skeletons, and it worked quite well. I just added the following npm scripts, and used npm run e2e to run the tests.

"protractor": "webdriver-manager update && protractor ./test/protractor.conf.js",
"e2e": "start-server-and-test :8080 protractor"

This PR is about when we are using webpack, right? What about built-in cli bundler?

You are right, this PR is about webpack only, and does not affect the cli-bundler. This means that the existing tasks will still continue to work for the cli-bundler.

3cp · 2019-07-20T23:10:31Z

@3cp Does supporting the au tasks means supporting the same set of arguments to these tasks as well? Does that not make everything more complex than before?

Don't have to. We can do very cheaply to send all the args to webpack. So the new webpack config has to support the old args as much as possible.

const additional = process.argv.slice(3); // get `--host xxx` from `au run --host xxx`.

if (additional.length) {
  // calls `npm start -- --host 127.0.0.1` which pass to webpack directly
  return (new NPM()).install(['--', ...additional], process.cwd(), 'start');
}

Sayan751 · 2019-07-21T04:46:16Z

@3cp That's nice, I can make the change

shahabganji · 2019-07-21T11:49:11Z

@Sayan751

Sounds great, just one question out of curiosity, for protractor We have to add these lines to run it in headless mode which has no conflict with your PR, I guess we should still pass --run right?, but for the cypress to run in headless mode, I think it was these lines, how is it possible to run cypress and protractor in headless mode?

And, one thing more, don't you think it would be great to keep the old version as well as adding this new approach? Kinda have the deprecated flag for the old approach, it is just a suggestion.

--EDIT: ------

And AFAIK, when we use cli, no package need to be installed globally, but cli, I think it would be better these npm scripts follow the same rule and reference the locally installed packages

"protractor": "./node_modules/.bin/webdriver-manager update \
    && ./node_modules/.bin/protractor ./test/protractor.conf.js",
"e2e": "./node_modules/.bin/start-server-and-test :8080 protractor"

cc: @3cp

3cp · 2019-07-24T23:43:52Z

LGTM. Waiting for #1140 fix on master head.

skeleton/webpack/aurelia_project/tasks/run.ext

skeleton/webpack/aurelia_project/tasks/build.ext

Additionally small refactor in package manager to easily export NPM and Yarn. Added a new method `run` in base package manager.

3cp · 2019-08-01T00:59:45Z

I will test this one locally first before merging other PRs.

Sayan751 mentioned this pull request Jul 13, 2019

CLI-less webpack build #1105

Closed

3cp reviewed Jul 16, 2019

View reviewed changes

skeleton/webpack/gulpFile.js Outdated Show resolved Hide resolved

skeleton/scaffold-navigation/src/main.ext Outdated Show resolved Hide resolved

skeleton/webpack/webpack.config.js Show resolved Hide resolved

chore(webpack): post review changes

a909a5a

- Gulp is removed from webpack project; del is replaced by clean-webpack-plugin - Environment import is retained for cli-buindler

chore(webpack): added gulp back to webpack projects

c03991f

The dependency to gulp is added back for the webpack projects as there are still some au tasks that depends on gulp. Additionally, removed environment task.

chore(webpack): reactivation of au task as alias

76ffaef

3cp requested changes Jul 25, 2019

View reviewed changes

skeleton/webpack/aurelia_project/tasks/run.ext Outdated Show resolved Hide resolved

3cp reviewed Jul 29, 2019

View reviewed changes

skeleton/webpack/aurelia_project/tasks/build.ext Outdated Show resolved Hide resolved

fix(webpack): new release check

a217fec

Additionally small refactor in package manager to easily export NPM and Yarn. Added a new method `run` in base package manager.

3cp merged commit a217fec into aurelia:master Aug 1, 2019

3cp mentioned this pull request Aug 19, 2019

Aurelia CLI run webpack task assumes config.entry.app exists #1144

Open

3cp mentioned this pull request Sep 7, 2019

Add `--extractCss' flag to CLI Webpack #983

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(webpack):enable CLI-less webpack build + run #1137

feat(webpack):enable CLI-less webpack build + run #1137

Sayan751 commented Jul 13, 2019

jwx commented Jul 13, 2019

3cp commented Jul 17, 2019

Sayan751 commented Jul 17, 2019

3cp commented Jul 17, 2019 •

edited

Loading

3cp commented Jul 17, 2019

3cp commented Jul 18, 2019

3cp commented Jul 18, 2019

Sayan751 commented Jul 18, 2019

Sayan751 commented Jul 18, 2019

jwx commented Jul 18, 2019

Sayan751 commented Jul 18, 2019

3cp commented Jul 19, 2019

3cp commented Jul 19, 2019

shahabganji commented Jul 19, 2019

Sayan751 commented Jul 19, 2019

Sayan751 commented Jul 19, 2019

shahabganji commented Jul 20, 2019

Sayan751 commented Jul 20, 2019

3cp commented Jul 20, 2019

Sayan751 commented Jul 21, 2019

shahabganji commented Jul 21, 2019 •

edited

Loading

3cp commented Jul 24, 2019

3cp commented Aug 1, 2019

feat(webpack):enable CLI-less webpack build + run #1137

feat(webpack):enable CLI-less webpack build + run #1137

Conversation

Sayan751 commented Jul 13, 2019

jwx commented Jul 13, 2019

3cp commented Jul 17, 2019

Sayan751 commented Jul 17, 2019

3cp commented Jul 17, 2019 • edited Loading

3cp commented Jul 17, 2019

3cp commented Jul 18, 2019

3cp commented Jul 18, 2019

Sayan751 commented Jul 18, 2019

Sayan751 commented Jul 18, 2019

jwx commented Jul 18, 2019

Sayan751 commented Jul 18, 2019

3cp commented Jul 19, 2019

3cp commented Jul 19, 2019

shahabganji commented Jul 19, 2019

Sayan751 commented Jul 19, 2019

Sayan751 commented Jul 19, 2019

shahabganji commented Jul 20, 2019

Sayan751 commented Jul 20, 2019

3cp commented Jul 20, 2019

Sayan751 commented Jul 21, 2019

shahabganji commented Jul 21, 2019 • edited Loading

3cp commented Jul 24, 2019

3cp commented Aug 1, 2019

3cp commented Jul 17, 2019 •

edited

Loading

shahabganji commented Jul 21, 2019 •

edited

Loading