search

ramandeep-singh-1983 / ngx-powerbi

TypeScript library for embedding Power BI assets (reports/dashboards/tiles) in your application. This TypeScript library is built on top of the official powerbi-client library provided by Microsoft.
MIT License
36 stars 22 forks source link

Angular update version 14 #67

Closed vsmolyak closed 2 years ago

bjorn-einar-bjartnes-4ss commented 2 years ago

Would like this compatibility too, but it might be better to use || in versions to allow for them both to work? Sort of like 

   "@angular/compiler": "^14.1.0 || ^13.2.5",
vsmolyak commented 2 years ago

@bjorn-einar-bjartnes-4ss Why would you want to update only compiler version? Also would it be compatible with es target? In this pull request it was updated from es2015 to es2020

bjartwolf commented 2 years ago

Sorry, I was unclear, I did not mean compiler version pr se, I was just pointing out that it might be possible to target multiple major version with using || instead of building a specific package for each version of angular as long as they are compatible (as it seems to be, we use this package with angular 14 now - the only issue I have is that I have to run with --legacy-peer-deps to get npm to not complain.

bjorn-einar-bjartnes-4ss commented 2 years ago

Anyway, it would be good to see this merged. It would solve https://github.com/ramandeep-singh-1983/ngx-powerbi/issues/66. The other way forward is to make a new package.

vsmolyak commented 2 years ago

Yes, i am a bit confused by how compiler's older version would be compatible with the rest of toolkit. If you like to extend this then better to create a new PR. Also i'm not sure how to test it.

vsmolyak commented 2 years ago

@ramandeep-singh-1983 maybe you could give some feedback?

barco-ramsi commented 2 years ago

@vsmolyak have you tested the package with your changes? I am involved somewhere so won't be able to test it out myself. So, if you say it's working, I will approve the PR.

@bjorn-einar-bjartnes-4ss: feel free to create a new PR with your suggestion and let me know if it's working.

bjartwolf commented 2 years ago

What I know is working, is that the library as is does run with Angular 14 (we use it in production with Angular 14, but we have to ignore the warnings from npm)

vsmolyak commented 2 years ago

@ramandeep-singh-1983 i tested the updated package in my project and it was working.

bjorn-einar-bjartnes-4ss commented 2 years ago

Any chance you could merge this @barco-ramsi / @ramandeep-singh-1983 ? I will immediately try to test it in our project and report back.

ramandeep-singh-1983 commented 2 years ago

Sorry for the delay guys. I have approved and merged the PR now. I'll try to release the new package to npm at the earliest.

bjorn-einar-bjartnes-4ss commented 2 years ago

Thanks @ramandeep-singh-1983

(It could be worth investigating setting up a https://github.com/sponsors here, I can not guarantee anything as I would have to ask for approval, but just keeping it up to date with current angular major versions could make it possible to set up something... )

ramandeep-singh-1983 commented 2 years ago

@bjorn-einar-bjartnes-4ss thanks, I'll check out the link you shared.

bjorn-einar-bjartnes-4ss commented 2 years ago

Sorry for the delay guys. I have approved and merged the PR now. I'll try to release the new package to npm at the earliest.

Any chance to push this to npm? https://www.npmjs.com/package/ngx-powerbi is still not updated?

barco-ramsi commented 2 years ago

Apologies again, it was already on the back of my mind but as my laptop wasn't set up correctly, I have been ignoring it. I'll try to do it in the coming week.

ramandeep-singh-1983 commented 2 years ago

Just pushed it to npm.