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

Angular 17 #1757

Merged
merged 17 commits into from
Jan 27, 2024
Merged

Angular 17 #1757

merged 17 commits into from
Jan 27, 2024

Conversation

santam85
Copy link
Contributor

@santam85 santam85 commented Nov 8, 2023

Upgrade build to Angular 17

@santam85 santam85 marked this pull request as draft November 9, 2023 09:52
@santam85 santam85 marked this pull request as ready for review January 23, 2024 13:50
@PowerKiKi
Copy link
Contributor

@santam85 the build failed. It seems the screenshots changed a bit. Are you able to fix that ?

@valorkin, assuming the CI passes, would you have any other feedback before merging this ?

@santam85
Copy link
Contributor Author

I'll push the new screenshots when I have a minute (requires some time as it has to run in a docker container and having trouble with that). There are a few more issues I'd like to address with the unit tests for the doc site, but that doesn't block releasing of an Angular 17 version of the lib. Would be nice if someone could contribute the modernization part, IE: switching to signals, module-less etc., so that it's not only a bump in the peer dependencies.

@PowerKiKi
Copy link
Contributor

I am interested in standalone, so I'd create a PR for it. But it seems better to wait for this one to be merged, because it it contains quite a few changes that I'd rather already have when working on my PR.

@santam85 santam85 merged commit f1c2391 into master Jan 27, 2024
1 check passed
@santam85 santam85 deleted the angular-17 branch January 27, 2024 11:30
@santam85
Copy link
Contributor Author

@PowerKiKi PR is merged, but I will wait to release until some work is done to modernize the lib for 17 and fix the unit testing issues, thanks for having a look at that! 🙏

@PowerKiKi
Copy link
Contributor

Thanks for the heads up. I had a quick look but npm run start will fail with errors similar to the following. Since this was introduced in this PR, maybe there is something I should know ? Does this code require node 20 (I am on 18) ?

✘ [ERROR] Could not resolve "../bar-chart/bar-chart.component.txt"

    apps/ng2-charts-demo/src/app/chart-host/chart-host.component.ts:3:18:
      3 │ import barTs from '../bar-chart/bar-chart.component.txt';
        ╵                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@santam85
Copy link
Contributor Author

there is a prebuild:prod script you can run to generate those

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.

None yet

2 participants