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

arturovt · 2020-04-12T00:17:01Z

No description provided.

arturovt · 2020-04-12T00:18:21Z

src/src/single-spa-angular.ts

@@ -15,7 +15,7 @@ const defaultOpts = {
  updateFunction: () => Promise.resolve(),
 };

-export default function singleSpaAngular(userOpts: SingleSpaAngularOpts): LifeCycles {
+export function singleSpaAngular(userOpts: SingleSpaAngularOpts): LifeCycles {


⚠️ breaking change for @4 because angular's package format and ng-packagr don't allow default exports.

:'( that sucks

joeldenning

Thanks for your work on this. I honestly don't understand everything here, since I don't really know Angular schematics, ngpackagr, or really much about Angular at all :)

My biggest question is why we have multiple package.json's now - it seems like the root package.json is set up monorepo style, but then it has all of the dependencies and build scripts which is not how monorepos work. Why does only the browser lib have its own package.json instead of also the schematics and webpack libs?

joeldenning · 2020-04-13T15:57:04Z

package.json

-  "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",


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

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.

joeldenning · 2020-04-13T15:59:04Z

package.json

-    "clean": "rimraf lib parcel",
-    "test": "jest",
+    "clean": "rimraf lib pacel",
+    "build": "run-s clean build:single-spa-angular build:webpack build:schematics",


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:*'",

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

joeldenning · 2020-04-13T16:02:24Z

src/src/single-spa-angular.ts

@@ -15,7 +15,7 @@ const defaultOpts = {
  updateFunction: () => Promise.resolve(),
 };

-export default function singleSpaAngular(userOpts: SingleSpaAngularOpts): LifeCycles {
+export function singleSpaAngular(userOpts: SingleSpaAngularOpts): LifeCycles {


:'( that sucks

joeldenning · 2020-04-13T16:05:22Z

package.json

    "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",


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:*'",

joeldenning · 2020-04-13T16:06:28Z

package.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",


Why is there a lib/lib directory?

webpack is emitted there

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

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

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

joeldenning · 2020-04-13T16:07:17Z

package.json

    "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",


Why is this needed?

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

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

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

arturovt · 2020-04-13T21:13:30Z

@joeldenning I would like you to pull changes and test them locally. That would be great since I don't wanna introduce any breaking changes. You can see something that I don't see :D

joeldenning · 2020-04-13T23:18:49Z

I'll take a look tomorrow

joeldenning · 2020-04-14T23:23:24Z

I didn't get to this today. Will try for tomorrow. Today, tomorrow, and Thursday I am doing full-time consulting with a company. I recently quit my job to try to take my work on open source full time, and do consulting a few days a month to help pay the bills. This week happens to be the week where I'm doing that consulting. So I'll try to fit it in in the next couple days, but if not then I will definitely take a look at this by Friday

joeldenning · 2020-04-17T15:45:48Z

I am looking at this now. Have a few questions that I'll share in a PR with a few small changes. If you're available and have time to chat over Slack, let's talk in the single-spa slack workspace.

arturovt · 2020-04-17T15:47:26Z

@joeldenning I'm having a Friday release. I will be available tomorrow.

joeldenning · 2020-04-17T15:50:11Z

👍 sounds good - I will send a PR up with some small changes and also have a list of questions in it.

build: apply Angular package format and ng-packagr

2874847

arturovt commented Apr 12, 2020

View reviewed changes

arturovt added 2 commits April 12, 2020 15:53

build: remove options from webpack config

b8efae3

build: remove options from webpack config

c8b608f

arturovt marked this pull request as ready for review April 12, 2020 12:56

arturovt requested a review from joeldenning April 12, 2020 12:56

fix: peer dep

38a2a34

joeldenning reviewed Apr 13, 2020

View reviewed changes

joeldenning approved these changes Apr 13, 2020

View reviewed changes

joeldenning merged commit 6a7d487 into single-spa:master Apr 13, 2020

arturovt deleted the ng-packagr branch April 13, 2020 21:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

arturovt commented Apr 12, 2020

arturovt Apr 12, 2020

joeldenning Apr 13, 2020

joeldenning left a comment

joeldenning Apr 13, 2020

arturovt Apr 13, 2020

joeldenning Apr 13, 2020

arturovt Apr 13, 2020

joeldenning Apr 13, 2020

joeldenning Apr 13, 2020

joeldenning Apr 13, 2020

arturovt Apr 13, 2020

joeldenning Apr 13, 2020

joeldenning Apr 13, 2020

arturovt Apr 13, 2020

joeldenning Apr 13, 2020

arturovt Apr 13, 2020

joeldenning Apr 13, 2020

arturovt Apr 13, 2020 •

edited

arturovt commented Apr 13, 2020 •

edited

joeldenning commented Apr 13, 2020

joeldenning commented Apr 14, 2020

joeldenning commented Apr 17, 2020

arturovt commented Apr 17, 2020

joeldenning commented Apr 17, 2020

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

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

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

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

Conversation

arturovt commented Apr 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joeldenning left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arturovt Apr 13, 2020 • edited

Choose a reason for hiding this comment

arturovt commented Apr 13, 2020 • edited

joeldenning commented Apr 13, 2020

joeldenning commented Apr 14, 2020

joeldenning commented Apr 17, 2020

arturovt commented Apr 17, 2020

joeldenning commented Apr 17, 2020

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

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

arturovt Apr 13, 2020 •

edited

arturovt commented Apr 13, 2020 •

edited