Skip to content

build: use shared logic to build packages #4202

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

Merged
merged 2 commits into from
Apr 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions DEV_ENVIRONMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
5. From the root of the project, run `npm install`.


To build the components in dev mode, run `gulp build:components`.
To build the components in release mode, run `gulp build:release`
To build Material in dev mode, run `gulp material:build`.
To build Material in release mode, run `gulp material:build-release`

To bring up a local server, run `gulp serve:devapp`. This will automatically watch for changes
and rebuild. The browser should refresh automatically when changes are made.
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"url": "https://github.com/angular/material2.git"
},
"scripts": {
"build": "gulp build:release",
"build": "gulp material:build-release:clean",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't has to happen in this PR, but can we swap the rule names so that they're consistently verb:subject again? E.g.,
build:material
watch:material
serve:devapp
release:cdk
test:cdk
package:material (create the release package)
publish:material (publishes to npm)
clean:cdk

It would also be good (in a follow-up PR) to have just gulp build, gulp test, etc. that do material, cdk, and examples.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about that. It's very nice to have like all tasks that belong to a different "package" / "group" be prefixed with the actual name of it.

cdk:clean-build
cdk:build
cdk:build-release
cdk:watch

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think of it as "I want to perform some action. That action will affect xxx". Overall I like the consistency of "verb:action" more than organizing the tasks by target

Copy link
Member Author

@devversion devversion Apr 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I see that it might be nicer when building different packages on their own. I personally vote for organizing the tasks by target.

It just feels not very consistent and convenient when naming stuff that won't be available in other tasks.

  • copy-assets:demo-app.
  • bundle-theme-scss:material

I would be fine revisiting later in another PR or issue.

"demo-app": "gulp serve:devapp",
"test": "gulp test",
"tslint": "gulp lint",
Expand Down
4 changes: 2 additions & 2 deletions scripts/closure-compiler/build-devapp-bundle.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ set -e -o pipefail
cd $(dirname $0)/../..


# Build a release of the library and of the CDK package.
$(npm bin)/gulp build:release
# Build a release of material and of the CDK package.
$(npm bin)/gulp material:build-release:clean
$(npm bin)/gulp cdk:build-release

# Build demo-app with ES2015 modules. Closure compiler is then able to parse imports.
Expand Down
2 changes: 1 addition & 1 deletion scripts/release/publish-build-artifacts.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ repoUrl="https://github.com/angular/material2-builds.git"
repoDir="tmp/$repoName"

# Create a release of the current repository.
$(npm bin)/gulp build:release
$(npm bin)/gulp material:build-release:clean

# Prepare cloning the builds repository
rm -rf $repoDir
Expand Down
2 changes: 1 addition & 1 deletion src/material-examples/tsconfig-build.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"experimentalDecorators": true,
"module": "es2015",
"moduleResolution": "node",
"outDir": "../../dist/packages/examples",
"outDir": "../../dist/packages/material-examples",
"rootDir": ".",
"sourceMap": true,
"inlineSources": true,
Expand Down
13 changes: 9 additions & 4 deletions tools/gulp/gulpfile.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
import {createPackageBuildTasks} from './util/package-tasks';

/** Create gulp tasks to build the different packages in the project. */
createPackageBuildTasks('cdk');
createPackageBuildTasks('material', ['cdk']);
createPackageBuildTasks('material-examples', ['material']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just call this one 'examples' for brevity

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name in the package.json will be @angular/material-examples. Before I went with material-examples I had another property on createPackageBuildTasks that allows you to specify the task-prefix.

createPackageBuildTasks('material-examples', 'examples', ['material']);

But this just causes confusion because the package and task name are different. Actually the same for lib (= material).

We should try to be as consistent as possible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The project/repo is already called "material", though; I'd say that part of the name is implicit. As someone working on it, my mental model is just "I want to build the examples" -> "build:examples".

Copy link
Member Author

@devversion devversion Apr 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally see what you mean. My only concern is that it is very confusing to have a mismatch of package-name / task-name / source directory name.


import './tasks/ci';
import './tasks/clean';
import './tasks/default';
import './tasks/development';
import './tasks/docs';
import './tasks/e2e';
import './tasks/lint';
import './tasks/release';
import './tasks/publish';
import './tasks/screenshots';
import './tasks/unit-test';
import './tasks/aot';
import './tasks/payload';
import './tasks/coverage';
import './tasks/library';
import './tasks/examples';
import './tasks/cdk';
import './tasks/material-release';
4 changes: 2 additions & 2 deletions tools/gulp/tasks/aot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ import {join} from 'path';

const tsconfigFile = join(DIST_DEMOAPP, 'tsconfig-aot.json');

/** Builds the demo-app and library. To be able to run NGC, apply the metadata workaround. */
/** Builds the demo-app and material. To be able to run NGC, apply the metadata workaround. */
task('aot:deps', sequenceTask(
'build:devapp',
[':package:release', 'cdk:build-release'],
['material:build-release', 'cdk:build-release'],
'aot:copy-release'
));

Expand Down
56 changes: 0 additions & 56 deletions tools/gulp/tasks/cdk.ts

This file was deleted.

2 changes: 1 addition & 1 deletion tools/gulp/tasks/development.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,5 @@ task('build:devapp', buildAppTask('devapp'));
task(':serve:devapp', serverTask(outDir, true));

task('serve:devapp', ['build:devapp'], sequenceTask(
[':serve:devapp', 'library:watch', ':watch:devapp']
[':serve:devapp', 'material:watch', ':watch:devapp']
));
2 changes: 1 addition & 1 deletion tools/gulp/tasks/e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ task('serve:e2eapp', sequenceTask('build:e2eapp', ':serve:e2eapp'));
* [Watch task] Builds and serves e2e app, rebuilding whenever the sources change.
* This should only be used when running e2e tests locally.
*/
task('serve:e2eapp:watch', ['serve:e2eapp', 'library:watch', ':watch:e2eapp']);
task('serve:e2eapp:watch', ['serve:e2eapp', 'material:watch', ':watch:e2eapp']);

/**
* Builds and serves the e2e-app and runs protractor once the e2e-app is ready.
Expand Down
46 changes: 0 additions & 46 deletions tools/gulp/tasks/examples.ts

This file was deleted.

56 changes: 0 additions & 56 deletions tools/gulp/tasks/library.ts

This file was deleted.

2 changes: 1 addition & 1 deletion tools/gulp/tasks/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {DIST_MATERIAL} from '../constants';
gulp.task('lint', ['tslint', 'stylelint', 'madge', 'dashboardlint']);

/** Task that runs madge to detect circular dependencies. */
gulp.task('madge', ['library:clean-build'], execNodeTask('madge', ['--circular', DIST_MATERIAL]));
gulp.task('madge', ['material:clean-build'], execNodeTask('madge', ['--circular', DIST_MATERIAL]));

/** Task to lint Angular Material's scss stylesheets. */
gulp.task('stylelint', execNodeTask(
Expand Down
53 changes: 53 additions & 0 deletions tools/gulp/tasks/material-release.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import {task, src, dest} from 'gulp';
import {join} from 'path';
import {writeFileSync} from 'fs';
import {Bundler} from 'scss-bundle';
import {execNodeTask, sequenceTask} from '../util/task_helpers';
import {composeRelease} from '../util/package-build';
import {COMPONENTS_DIR, DIST_MATERIAL, DIST_RELEASES} from '../constants';

// There are no type definitions available for these imports.
const gulpRename = require('gulp-rename');

// Path to the release output of material.
const releasePath = join(DIST_RELEASES, 'material');
// The entry-point for the scss theming bundle.
const themingEntryPointPath = join(COMPONENTS_DIR, 'core', 'theming', '_all-theme.scss');
// Output path for the scss theming bundle.
const themingBundlePath = join(releasePath, '_theming.scss');
// Matches all pre-built theme css files
const prebuiltThemeGlob = join(DIST_MATERIAL, '**/theming/prebuilt/*.css');
// Matches all SCSS files in the library.
const allScssGlob = join(COMPONENTS_DIR, '**/*.scss');

/**
* Overwrite the release task for the material package. The material release will include special
* files, like a bundled theming SCSS file or all prebuilt themes.
*/
task('material:build-release', ['material:prepare-release'], () => composeRelease('material'));

/**
* Task that will build the material package. It will also copy all prebuilt themes and build
* a bundled SCSS file for theming
*/
task('material:prepare-release', sequenceTask(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you feel about naming tasks that are not meant to be called from the command line with a leading :?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had that before for a lot of different tasks. But I just don't feel it's necessary. Just makes it harder to remind, if something is "supposed" to be called from the CLI or not.

'material:build',
['material:copy-prebuilt-themes', 'material:bundle-theming-scss']
));

/** Copies all prebuilt themes into the release package under `prebuilt-themes/` */
task('material:copy-prebuilt-themes', () => {
src(prebuiltThemeGlob)
.pipe(gulpRename({dirname: ''}))
.pipe(dest(join(releasePath, 'prebuilt-themes')));
});

/** Bundles all scss requires for theming into a single scss file in the root of the package. */
task('material:bundle-theming-scss', () => {
// Instantiates the SCSS bundler and bundles all imports of the specified entry point SCSS file.
// A glob of all SCSS files in the library will be passed to the bundler. The bundler takes an
// array of globs, which will match SCSS files that will be only included once in the bundle.
new Bundler().Bundle(themingEntryPointPath, [allScssGlob]).then(result => {
writeFileSync(themingBundlePath, result.bundledContent);
});
});
4 changes: 2 additions & 2 deletions tools/gulp/tasks/payload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import {openFirebaseDashboardDatabase} from '../util/firebase';

const bundlesDir = join(DIST_ROOT, 'bundles');

/** Task which runs test against the size of whole library. */
task('payload', ['library:clean-build'], () => {
/** Task which runs test against the size of material. */
task('payload', ['material:clean-build'], () => {

let results = {
umd_kb: getBundleSize('material.umd.js'),
Expand Down
Loading