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

build: apply Angular package format and ng-packagr #167

Merged
merged 4 commits into from
Apr 13, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/single-spa-angular.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ jobs:
- name: Run ESLint
run: yarn lint

- name: Build packages
run: yarn build

- name: Run Jest
run: yarn test:ci

- name: Build packages
run: yarn build

- name: Install and build integration apps
run: |
yarn install:integration
Expand Down
1 change: 0 additions & 1 deletion .npmignore

This file was deleted.

2 changes: 1 addition & 1 deletion integration/shop/angular.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"builder": "@angular-builders/custom-webpack:browser",
"options": {
"customWebpackConfig": {
"path": "./webpack.config.ts"
"path": "./webpack.config.js"
},
"outputPath": "dist",
"index": "src/index.html",
Expand Down
3 changes: 2 additions & 1 deletion integration/shop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"build": "ng build",
"build:single-spa": "ng build --prod --deploy-url http://localhost:4200/",
"serve:single-spa": "ng serve --disable-host-check --port 4200 --deploy-url http://localhost:4200/ --live-reload false",
"preinstall": "cp -r ../../src/browser-lib ./lib",
"preinstall": "yarn --cwd ../../ cpx -v -C \"lib/**/*\" ./integration/shop/node_modules/single-spa-angular",
"postinstall": "ngcc"
},
"private": true,
Expand All @@ -20,6 +20,7 @@
"@angular/platform-browser": "file:../../node_modules/@angular/platform-browser",
"@angular/platform-browser-dynamic": "file:../../node_modules/@angular/platform-browser-dynamic",
"@angular/router": "file:../../node_modules/@angular/router",
"single-spa-angular": "file:./node_modules/single-spa-angular",
"rxjs": "file:../../node_modules/rxjs",
"tslib": "file:../../node_modules/tslib",
"zone.js": "file:../../node_modules/zone.js"
Expand Down
2 changes: 1 addition & 1 deletion integration/shop/src/main.single-spa.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { enableProdMode, NgZone } from '@angular/core';
import { Router } from '@angular/router';
import { platformBrowserDynamic } from '@angular/platform-browser-dynamic';
import singleSpaAngular, { getSingleSpaExtraProviders } from 'single-spa-angular';
import { singleSpaAngular, getSingleSpaExtraProviders } from 'single-spa-angular';

import { AppModule } from './app/app.module';
import { environment } from './environments/environment';
Expand Down
3 changes: 0 additions & 3 deletions integration/shop/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
"compileOnSave": false,
"compilerOptions": {
"baseUrl": "./",
"paths": {
"single-spa-angular": ["./lib/single-spa-angular.ts"],
},
"module": "esnext",
"moduleResolution": "node",
"target": "es2015",
Expand Down
7 changes: 7 additions & 0 deletions integration/shop/webpack.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
const singleSpaAngularWebpack = require('single-spa-angular/lib/webpack').default;

module.exports = config => {
config = singleSpaAngularWebpack(config);
config.output.library = 'shop';
return config;
};
9 changes: 0 additions & 9 deletions integration/shop/webpack.config.ts

This file was deleted.

3 changes: 3 additions & 0 deletions integration/shop/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6553,6 +6553,9 @@ simple-swizzle@^0.2.2:
dependencies:
is-arrayish "^0.3.1"

"single-spa-angular@file:./node_modules/single-spa-angular":
version "3.3.0"

slash@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/slash/-/slash-1.0.0.tgz#c41f2f6c39fc16d1cd17ad4b5d896114ae470d55"
Expand Down
77 changes: 40 additions & 37 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,28 +1,54 @@
{
"name": "single-spa-angular",
"version": "3.3.0",
"description": "Helpers for building single-spa applications which use Angular 2",
"main": "lib/browser-lib/single-spa-angular.js",
"schematics": "./schematics.json",
"version": "0.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

Is this now 0.0.0 because the repo has been converted to a monorepo with multiple package.jsons?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://github.com/ng-packagr/ng-packagr/blob/master/docs/DESIGN.md#folder-layout

Not monorepo. Basically the root package.json we use for such stuff as scripts, dependencies and husky/prettier.

There is another package.json inside the src folder that is taken by ng-packagr. ng-packagr automatically adds webpack fields like main/module/es2015/esm5 and typings field.

"description": "Helpers for building single-spa applications which use Angular 2+",
"repository": {
"type": "git",
"url": "git+https://github.com/single-spa/single-spa-angular.git"
},
"keywords": [
"single-spa",
"angular"
],
"author": "Joel Denning",
"maintainers": [
"Joel Denning",
"Artur Androsovych"
],
"license": "Apache-2.0",
"bugs": {
"url": "https://github.com/single-spa/single-spa-angular/issues"
},
"homepage": "https://github.com/single-spa/single-spa-angular#readme",
"private": true,
"scripts": {
"prepublishOnly": "yarn build && yarn test",
"build": "yarn clean && yarn build:webpack && yarn parcel && yarn spa && yarn :copy-schematic-files",
"clean": "rimraf lib parcel",
"test": "jest",
"clean": "rimraf lib pacel",
"build": "run-s clean build:single-spa-angular build:webpack build:schematics",
Copy link
Member

Choose a reason for hiding this comment

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

Do these all need to be sequential? Couldn't all the builds occur after the clean?

Suggested change
"build": "run-s clean build:single-spa-angular build:webpack build:schematics",
"build": "yarn clean && concurrently -n w: 'yarn:build:*'",

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Umm, yes. Basically build:single-spa-angular should be first, because it will create lib folder. Other scripts can be parallel.

"test": "yarn clean && yarn jest",
"lint": "eslint ./src/**/*.ts",
":copy-schematic-files": "cpx \"src/**/_files/**/**\" lib",
"parcel": "ngc -p tsconfig.parcel.json",
"spa": "tsc -p tsconfig.spa.json",
"build:webpack": "tsc -p tsconfig.node.json",
"// - SINGLE-SPA-ANGULAR": "Scripts for building single-spa-angular",
"build:single-spa-angular": "yarn ts-node tools/build",
"// - WEBPACK": "Scripts for building Webpack config transformer",
"prebuild:webpack": "rimraf lib/lib",
Copy link
Member

Choose a reason for hiding this comment

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

Why is there a lib/lib directory?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

webpack is emitted there

Copy link
Member

Choose a reason for hiding this comment

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

Why then is the import for webpack at single-spa-angular/lib/webpack instead of single-spa-angular/lib/lib/webpack?

https://github.com/single-spa/single-spa-angular/pull/167/files#diff-def93996c46300cb52885ed7c3584275R1

Copy link
Member

Choose a reason for hiding this comment

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

This question is the main one I'm waiting on before reviewing again to approve/merge ^

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The content of the lib folder is copied into node_modules/single-spa-angular folder

"build:webpack": "tsc -p tsconfig.webpack.json",
"// - SCHEMATICS": "Scripts for building schematics and copying its files",
"prebuild:schematics": "rimraf lib/schematics",
"build:schematics": "tsc -p tsconfig.schematics.json && run-s copy:schematics:*",
"copy:schematics:json": "cpx schematics/schematics.json lib/schematics",
"copy:schematics:schema": "cpx \"schematics/ng-add/schema*\" lib/schematics/ng-add",
"copy:schematics:files": "cpx \"schematics/ng-add/_files/**/**\" lib/schematics/ng-add/_files",
"format": "prettier './**/*' --write",
"// - INTEGRATION INSTALLS": "Install packages for integration apps",
"install:integration:shop": "yarn --cwd integration/shop install",
"install:integration:portal": "yarn --cwd integration/portal install",
"install:integration": "yarn install:integration:shop && yarn install:integration:portal",
"install:integration": "run-s install:integration:shop install:integration:portal",
Copy link
Member

Choose a reason for hiding this comment

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

Why sequential? Couldn't they run concurrently?

Suggested change
"install:integration": "run-s install:integration:shop install:integration:portal",
"install:integration": "concurrently -n w: 'yarn:install:integration:*'",

"// - INTEGRATION BUILDS": "Build apps that are required for E2E testing #requires yarn install:integration",
"build:integration:shop": "yarn --cwd integration/shop build:single-spa",
"build:integration:portal": "yarn --cwd integration/portal build:prod",
"build:integration": "yarn build:integration:shop && yarn build:integration:portal",
"build:integration": "run-s build:integration:shop build:integration:portal",
"// - APPS": "Serve apps that are required for E2E testing #requires yarn build:integration",
"start:integration": "concurrently -n w: yarn:start:integration:*",
"start:integration:shop": "yarn serve integration/shop/dist -s -l 4200 --cors",
Expand All @@ -36,32 +62,6 @@
"test:ci": "jest --runInBand",
"test:ci:integration": "yarn start-test start:integration 8080 cy:run"
},
"repository": {
"type": "git",
"url": "git+https://github.com/single-spa/single-spa-angular.git"
},
"keywords": [
"single-spa",
"angular"
],
"author": "Joel Denning",
"license": "Apache-2.0",
"bugs": {
"url": "https://github.com/single-spa/single-spa-angular/issues"
},
"homepage": "https://github.com/single-spa/single-spa-angular#readme",
"peerDependencies": {
"@angular-devkit/architect": ">=0.10.0",
"@angular-devkit/build-angular": ">=0.10.0",
"@angular-devkit/core": ">=0.10.0",
"@angular/compiler": ">=2",
"@angular/compiler-cli": ">=2",
"@angular/core": ">=2",
"rxjs": ">=6.0.0",
"single-spa": ">=4.0.0",
"webpack": ">=4.6.0",
"webpack-merge": "^4.2.1"
},
"devDependencies": {
"@angular-builders/custom-webpack": "~9.1.0",
"@angular-devkit/architect": "~0.901.0",
Expand Down Expand Up @@ -94,12 +94,15 @@
"husky": "^4.2.5",
"jest": "^24.5.0",
"lint-staged": "^10.1.3",
"ng-packagr": "^9.1.0",
"npm-run-all": "^4.1.5",
"prettier": "^2.0.4",
"rimraf": "^2.6.2",
"rxjs": "~6.5.5",
"serve": "^11.3.0",
"start-server-and-test": "^1.10.11",
"ts-jest": "^24.0.0",
"tsickle": "^0.38.1",
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's a part of ng-packagr, it adds jsdoc annotations for the closure compiler.

Copy link
Member

Choose a reason for hiding this comment

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

If it's part of ng-packagr, then why do we need to install it separately from ng-packagr?

Copy link
Collaborator Author

@arturovt arturovt Apr 13, 2020

Choose a reason for hiding this comment

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

Because it's not a peer dependency of ng-packagr, but it does require that package (I wasn't the author of such solution 🙂 )

"tslib": "^1.11.1",
"typescript": "3.7.5",
"zone.js": "~0.10.3"
Expand Down
10 changes: 0 additions & 10 deletions schematics.json

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
const singleSpaAngularWebpack = require('single-spa-angular/lib/webpack').default;

module.exports = (angularWebpackConfig, options) => {
const singleSpaWebpackConfig = singleSpaAngularWebpack(angularWebpackConfig, options);
module.exports = config => {
const singleSpaWebpackConfig = singleSpaAngularWebpack(config);

// Feel free to modify this webpack config however you'd like to
return singleSpaWebpackConfig;
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { platformBrowserDynamic } from '@angular/platform-browser-dynamic';<% if
import { Router } from '@angular/router';<% } if (usingBrowserAnimationsModule) { %>
import { ɵAnimationEngine as AnimationEngine } from '@angular/animations/browser';<% } %>

<% if (routing) { %>import singleSpaAngular, { getSingleSpaExtraProviders } from 'single-spa-angular';
<% } else { %>import singleSpaAngular from 'single-spa-angular';<% } %>
<% if (routing) { %>import { singleSpaAngular, getSingleSpaExtraProviders } from 'single-spa-angular';
<% } else { %>import { singleSpaAngular } from 'single-spa-angular';<% } %>

import { AppModule } from './app/app.module';
import { environment } from './environments/environment';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { SchematicTestRunner, UnitTestTree } from '@angular-devkit/schematics/testing';
import { join } from 'path';

const collectionPath = join(__dirname, '../../../schematics.json');
const collectionPath = join(__dirname, '../schematics.json');

const workspaceOptions = {
name: 'workspace',
Expand Down Expand Up @@ -51,7 +51,7 @@ describe('ng-add', () => {
.runSchematicAsync('ng-add', { project: 'second-cool-app' }, workspaceTree)
.toPromise();

const { scripts } = JSON.parse(tree.get('/package.json').content.toString());
const { scripts } = JSON.parse(tree.get('/package.json')!.content.toString());

// Assert
expect(scripts['build:single-spa:first-cool-app']).toBe(
Expand Down Expand Up @@ -80,7 +80,7 @@ describe('ng-add', () => {
.runSchematicAsync('ng-add', { project: 'additional-project' }, workspaceTree)
.toPromise();

const { scripts } = JSON.parse(tree.get('/package.json').content.toString());
const { scripts } = JSON.parse(tree.get('/package.json')!.content.toString());

// Arrange
expect(scripts['build:single-spa']).toBe('ng build --prod --deploy-url http://localhost:4200/');
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { NodeDependencyType, NodeDependency } from '@schematics/angular/utility/
export function getSingleSpaAngularDependency(): NodeDependency {
return {
name: 'single-spa-angular',
version: require('../../../package.json').version,
version: require('../../package.json').version,
overwrite: false,
type: NodeDependencyType.Default,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { join } from 'path';

import { Schema as NgAddOptions } from './schema';

const collectionPath = join(__dirname, '../../../schematics.json');
const collectionPath = join(__dirname, '../schematics.json');

const workspaceOptions = {
name: 'ss-workspace',
Expand Down
18 changes: 11 additions & 7 deletions src/schematics/ng-add/index.ts → schematics/ng-add/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,20 +108,24 @@ export function updateConfiguration(options: NgAddOptions) {
};
}

function updateProjectOldAngular(context, clientProject, project) {
function updateProjectOldAngular(
context: SchematicContext,
clientProject: WorkspaceProject,
project: any,
) {
context.logger.info('Using single-spa-angular builder for Angular versions before 8');

// Copy configuration from build architect
clientProject.architect['single-spa'] = clientProject.architect.build;
clientProject.architect['single-spa'].builder = 'single-spa-angular:build';
clientProject.architect[
clientProject!.architect!['single-spa'] = clientProject.architect!.build;
clientProject!.architect!['single-spa'].builder = 'single-spa-angular:build';
clientProject!.architect![
'single-spa'
].options.main = `${project.workspace.sourceRoot}/main.single-spa.ts`;

// Copy configuration from the serve architect
clientProject.architect['single-spa-serve'] = clientProject.architect.serve;
clientProject.architect['single-spa-serve'].builder = 'single-spa-angular:dev-server';
clientProject.architect['single-spa-serve'].options.browserTarget = `${project.name}:single-spa`;
clientProject.architect!['single-spa-serve'] = clientProject.architect!.serve;
clientProject.architect!['single-spa-serve'].builder = 'single-spa-angular:dev-server';
clientProject.architect!['single-spa-serve'].options.browserTarget = `${project.name}:single-spa`;
}

function updateProjectNewAngular(context: SchematicContext, clientProject: WorkspaceProject): void {
Expand Down
File renamed without changes.
File renamed without changes.
10 changes: 10 additions & 0 deletions schematics/schematics.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"$schema": "../node_modules/@angular-devkit/schematics/collection-schema.json",
"schematics": {
"ng-add": {
"description": "Adds single-spa-angular to application",
"schema": "./ng-add/schema.json",
"factory": "./ng-add"
}
}
}
4 changes: 4 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/**
* The public api for consumers of single-spa-angular
*/
export * from './src/public_api';
38 changes: 38 additions & 0 deletions src/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{
"$schema": "../node_modules/ng-packagr/ng-package.schema.json",
"name": "single-spa-angular",
"version": "3.3.0",
"description": "Helpers for building single-spa applications which use Angular 2",
"schematics": "./schematics/schematics.json",
"repository": {
"type": "git",
"url": "git+https://github.com/single-spa/single-spa-angular.git"
},
"keywords": [
"single-spa",
"angular"
],
"author": "Joel Denning",
"maintainers": [
"Joel Denning",
"Artur Androsovych"
],
"license": "Apache-2.0",
"bugs": {
"url": "https://github.com/single-spa/single-spa-angular/issues"
},
"homepage": "https://github.com/single-spa/single-spa-angular#readme",
"sideEffects": false,
"peerDependencies": {
"@angular/core": ">=9",
"single-spa": ">=4.0.0"
},
"ngPackage": {
"lib": {
"umdId": "single-spa-angular",
"flatModuleFile": "single-spa-angular",
"entryFile": "index.ts"
},
"dest": "../lib"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export class SingleSpaPlatformLocation extends ɵBrowserPlatformLocation {
* dependencies from the currently active injector, but it's a feature
* of Angular 8+. Should be used when we drop support for older versions.
*/
private ngZone: NgZone;
private ngZone!: NgZone;

onPopState(fn: (event: LocationChangeEvent) => void): void {
super.onPopState(event => {
Expand Down
File renamed without changes.
2 changes: 2 additions & 0 deletions src/src/public_api.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export { singleSpaAngular } from './single-spa-angular';
export { getSingleSpaExtraProviders } from './extra-providers';