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

Building Angular packages with ng-packagr #2236

Merged
merged 7 commits into from
Jan 5, 2024

Conversation

JBBianchi
Copy link
Contributor

This PR aims to replace the use of ngc by ng-packagr when building the angular packages.

In other words, the libraries don't provide a cjs output anymore, only (partially compiled) *esm.

In order to tidy the output, the sources have been moved to a library sub folder and exposed via a commonly used public_api file.

The example for the angular material renderer has also been modified to import the output of the library instead of the sources. This allows to actually validate that the package can be loaded and fully compiled (rather than rebuilding everything, the lib + the app, all at once). Therefore, the use of rollup has been dropped in favor of the internal angular browser builder.

Related to:

Copy link

netlify bot commented Jan 3, 2024

Deploy Preview for jsonforms-examples ready!

Name Link
🔨 Latest commit 153367c
🔍 Latest deploy log https://app.netlify.com/sites/jsonforms-examples/deploys/6597e13387444a00087af1c3
😎 Deploy Preview https://deploy-preview-2236--jsonforms-examples.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@coveralls
Copy link

coveralls commented Jan 4, 2024

Coverage Status

coverage: 83.078%. remained the same
when pulling 153367c on JBBianchi:ngpackagr
into 0e4f7ed on eclipsesource:master.

@lucas-koehler lucas-koehler self-requested a review January 5, 2024 09:02
Copy link
Contributor

@lucas-koehler lucas-koehler left a comment

Choose a reason for hiding this comment

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

Hi @JBBianchi ,
thank you for another great contribution ❤️
The changes already look pretty good to me. Code-wise I have only two very minor comments inline.

Unfortunately, the dev script in the angular-material package does not work for me.
What I did:

  1. Clean repo: git clean -dxf
  2. pnpm run build
  3. cd packages/angular-material
  4. pnpm run dev

This resulted in the following output:

Error output
~/Git/jsonforms/packages/angular-material  ➦ 189aec39  pnpm run dev

> @jsonforms/angular-material@3.2.0-beta.0 dev /home/lucas/Git/jsonforms/packages/angular-material
> pnpm run build:examples-app && npx http-server ./example/dist/ -c-1 -o


> @jsonforms/angular-material@3.2.0-beta.0 build:examples-app /home/lucas/Git/jsonforms/packages/angular-material
> pnpm run build && node ./build-example.js


> @jsonforms/angular-material@3.2.0-beta.0 build /home/lucas/Git/jsonforms/packages/angular-material
> node ./build-package.js

Building Angular Package

------------------------------------------------------------------------------
Building entry point '@jsonforms/angular-material'
------------------------------------------------------------------------------
✔ Compiling with Angular sources in Ivy partial compilation mode.
✔ Writing FESM bundles
✔ Copying assets
ℹ Removing scripts section in package.json as it's considered a potential security vulnerability.
ℹ Removing devDependencies section in package.json.
✔ Writing package manifest
✔ Built @jsonforms/angular-material

------------------------------------------------------------------------------
Built Angular Package
 - from: /home/lucas/Git/jsonforms/packages/angular-material
 - to:   /home/lucas/Git/jsonforms/packages/angular-material/lib
------------------------------------------------------------------------------

Build at: 2024-01-05T09:39:19.816Z - Time: 3394ms

node:internal/modules/cjs/loader:1137
  throw err;
  ^

Error: Cannot find module '@angular-devkit/core'
Require stack:
- /home/lucas/Git/jsonforms/packages/angular-material/build-example.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1134:15)
    at Module._load (node:internal/modules/cjs/loader:975:27)
    at Module.require (node:internal/modules/cjs/loader:1225:19)
    at require (node:internal/modules/helpers:177:18)
    at Object.<anonymous> (/home/lucas/Git/jsonforms/packages/angular-material/build-example.js:2:16)
    at Module._compile (node:internal/modules/cjs/loader:1356:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1414:10)
    at Module.load (node:internal/modules/cjs/loader:1197:32)
    at Module._load (node:internal/modules/cjs/loader:1013:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:128:12) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/home/lucas/Git/jsonforms/packages/angular-material/build-example.js'
  ]
}

Node.js v18.19.0
 ELIFECYCLE  Command failed with exit code 1.
 ELIFECYCLE  Command failed with exit code 1.

packages/angular-material/test/tsconfig.test.json Outdated Show resolved Hide resolved
@JBBianchi
Copy link
Contributor Author

Hi @JBBianchi , thank you for another great contribution ❤️ The changes already look pretty good to me. Code-wise I have only two very minor comments inline.

Unfortunately, the dev script in the angular-material package does not work for me. What I did:

1. Clean repo: git clean -dxf

2. pnpm run build

3. cd packages/angular-material

4. pnpm run dev

This resulted in the following output:
Error output

Instead of going to the package directory and running pnpm, I rely on lerna straight away from the workspace root:
npx lerna run dev --scope=@jsonforms/angular-material

It works for me and netlify, could you confirm it works on your end ?

JBBianchi and others added 2 commits January 5, 2024 11:10
- add missing dev dependency `@angular-devkit/core`
- add `@angular/core` as dev dependency to root package.json
- update pnpm-lock.yaml
@lucas-koehler
Copy link
Contributor

lucas-koehler commented Jan 5, 2024

Hi @JBBianchi ,
I tested your suggestion and it worked, thanks :) However, it is unexpected to me that it doesn't work in the directory. That might also be confusing for other devs because it works for the other packages with examples.

I briefly investigated and could fix it with 153367c.
The first issue was a missing dev dependency to @angular-devkit/core in angular-material. I think we should add this in any case as it directly requires this.

The second problem was that it could not find @angular/core. Apparently it checked the root package.json. After adding it as a dev dependency there, both ways of running the example work :)
Adding @angular/core is not the nicest solution but for me it is preferable to the dev script not working directly in the angular-material package.

Of course if you want, you can try to fix it without the @angular/core dev dependency at the root level but we can also leave it likes this :) Thanks again for your effort!

@JBBianchi
Copy link
Contributor Author

The first issue was a missing dev dependency to @angular-devkit/core in angular-material

Thanks for your improvement. I don't have any problem with having @angular/core in the root package.json

@sdirix WDYT ?

@sdirix
Copy link
Member

sdirix commented Jan 5, 2024

The first issue was a missing dev dependency to @angular-devkit/core in angular-material

Thanks for your improvement. I don't have any problem with having @angular/core in the root package.json

@sdirix WDYT ?

Adding "devDependencies" to make the tooling work more smoothly for developers is definitely fine.

Copy link
Contributor

@lucas-koehler lucas-koehler left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks again for the contribution ❤️

@lucas-koehler lucas-koehler linked an issue Jan 5, 2024 that may be closed by this pull request
@lucas-koehler lucas-koehler merged commit 3b58277 into eclipsesource:master Jan 5, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Library build of Angular packages
4 participants