refactor(all): Repackaging #2056

Merged
merged 21 commits into from Sep 12, 2016

Projects

None yet

5 participants

@hansl
Collaborator
hansl commented Sep 11, 2016 edited

Moving Angular-CLI into packages/ and building the angular-cli.

The published packages will now have JavaScript only (with their d.ts), instead of being a mix of TypeScript and JavaScript, and it won't have the hacks of having a custom require extension to load the TypeScript.

Our development flow will be more streamlined, tools are going to be better (no more undefined symbols that are actually defined), and the npm packages will boot up way faster (since there's no more compilation during runtime).

Locally nothing changes, use npm link and run your local ng. Using the npm releases though it should be all javascript from the release.

The E2E tests also now uses the dist folder output to install in the new E2E app, making sure the process works for an app published to npm.

Note: Even though we're following the lerna structure, we won't be using their tool anytime soon.

@googlebot googlebot added the cla: yes label Sep 11, 2016
@filipesilva filipesilva and 1 other commented on an outdated diff Sep 12, 2016
packages/angular-cli/tsconfig.json
@@ -25,9 +25,7 @@
}
},
"exclude": [
- "blueprints/*/files/**/*"
- ],
- "includes": [
- "./custom-typings.d.ts"
+ "blueprints/*/files/**/*",
+ "**/*.spec.ts"
@filipesilva
filipesilva Sep 12, 2016 Member

What spec files are these?

@hansl
hansl Sep 12, 2016 Collaborator

We'll move acceptance specs into this package later on. I can remove if you want.

@hansl
hansl Sep 12, 2016 Collaborator

I'm going to remove and put back. This tsconfig was copied from the one in ast-tools.

@Brocco Brocco commented on the diff Sep 12, 2016
packages/angular-cli/addon/index.js
+
+const config = require('../models/config');
+const path = require('path');
+
+module.exports = {
+ name: 'ng2',
+
+ config: function () {
+ this.project.ngConfig = this.project.ngConfig || config.CliConfig.fromProject().config;
+ },
+
+ blueprintsPath: function () {
+ return path.join(__dirname, '../blueprints');
+ },
+
+ includedCommands: function () {
@Brocco
Brocco Sep 12, 2016 Contributor

Missing help command

@hansl
hansl Sep 12, 2016 Collaborator

oopsie. done.

@filipesilva filipesilva and 1 other commented on an outdated diff Sep 12, 2016
plugins/karma.js
+module.exports = require('../packages/angular-cli/plugins/karma');
@filipesilva
filipesilva Sep 12, 2016 Member

If you want you can just completely remove plugin/karma.js to something you think is a better import. It was only introduced in the webpack version so it's fine if you move it.

@hansl
hansl Sep 12, 2016 Collaborator

No I think it's a great import. This file is necessary when you npm link your local version. I've added a comment.

@filipesilva filipesilva and 1 other commented on an outdated diff Sep 12, 2016
scripts/publish/build.js
+ from = path.relative(process.cwd(), from);
+ to = path.relative(process.cwd(), to);
+ return new Promise((resolve, reject) => {
+ const rs = fs.createReadStream(from);
+ const ws = fs.createWriteStream(to);
+
+ rs.on('error', reject);
+ ws.on('error', reject);
+ ws.on('close', resolve);
+
+ rs.pipe(ws);
+ });
+}
+
+
+function patch() {
@filipesilva
filipesilva Sep 12, 2016 Member

Can you add a comment explaining why this is necessary?

@hansl
hansl Sep 12, 2016 Collaborator

It's not anymore, so removed.

@filipesilva filipesilva and 1 other commented on an outdated diff Sep 12, 2016
scripts/publish/build.js
+ const name = path.relative(packagesRoot, pkg.root);
+
+ return promise.then(() => {
+ console.log(` ${name}`);
+ return npmRun.execSync(`tsc -p ${path.relative(process.cwd(), pkg.root)}`);
+ });
+ }, Promise.resolve());
+ })
+ .then(() => console.log('Copying uncompiled resources...'))
+ .then(() => glob(path.join(packagesRoot, '**/*')))
+ .then(files => {
+ console.log(` Found ${files.length} files...`);
+ return files
+ .map((fileName) => path.relative(packagesRoot, fileName))
+ .filter((fileName) => {
+ if (/^angular-cli.blueprints/.test(fileName)) {
@filipesilva
filipesilva Sep 12, 2016 Member

Shouldn't this be /^angular-cli\/blueprints/ instead? There doesn't seem to be any file with angular-cli.blueprints in it's name.

@hansl
hansl Sep 12, 2016 edited Collaborator

It's a regex, so . will match both / and \. Changed it to clearre.

@filipesilva
filipesilva Sep 12, 2016 Member

That didn't even occur to me, but of course it makes sense.

@filipesilva filipesilva and 1 other commented on an outdated diff Sep 12, 2016
tsconfig.json
@@ -6,7 +6,7 @@
"moduleResolution": "node",
"noEmitOnError": true,
"noImplicitAny": true,
- "outDir": "./dist/",
+ "outDir": "./dist/asd",
@hansl
hansl Sep 12, 2016 Collaborator

Part of a test. You should never use this tsconfig, it's only there for tooling.

@hansl hansl fixing comments
45f7824
@elvirdolic

What is the goal/reason for this mega PR.

@hansl
Collaborator
hansl commented Sep 12, 2016

Clarified the text.

@hansl hansl merged commit 1572270 into angular:master Sep 12, 2016

3 checks passed

cla/google All necessary CLAs are signed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hansl hansl deleted the hansl:repackaging branch Sep 12, 2016
@JohannesHoppe JohannesHoppe added a commit to angular-buch/angular-cli-ghpages that referenced this pull request Oct 8, 2016
@fmalcher @JohannesHoppe fmalcher + JohannesHoppe fixes addon for latest angular-cli (since angular/angular-cli#2056 bu…
…ild-webpack is now a JS file)
df45471
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment