mariusmuntean / ChartJs.Blazor

Brings Chart.js charts to Blazor
https://www.iheartblazor.com/
MIT License
676 stars 151 forks source link

Preview release (next nuget package) #127

Closed BorisGaliev closed 3 years ago

BorisGaliev commented 4 years ago

Describe your question

I wonder if it is planned to release a new version of a nuget package with last changes ? This could also be a preRelease version...

Which Blazor project type is your question related to?

Joelius300 commented 4 years ago

We've discussed releasing a preview version for 2.0 internally and I think it would make sense.

@mariusmuntean what commit do you think we should base this preview release on? In my opinion the latest (e930f50db02bb42c00c8f7db47419e4c19ea1c6e) would make sense but we should discuss it beforehand.
Also do you think it makes sense to setup a new pipeline or just do it manually? I'm in favour of doing it manually since we don't need to have it automated (unlike nightly builds for example).

I think we should release a nuget version called 2.0.0-preview1 with a respective tag v2.0.0-preview1 on e930f50db02bb42c00c8f7db47419e4c19ea1c6e. I would not push a commit with the adjusted package version but instead do that locally, pack it locally and publish it manually.

mariusmuntean commented 4 years ago

Setting up a new CI/CD pipeline for preview releases isn't a big deal, but we need to discuss the release strategy that we want. So I'm also leaning toward a manual build and publish.

Regarding which commit to take for the release: there is no reason not to choose the latest, but we also need to update the README.MD to reflect the API changes.

Joelius300 commented 4 years ago

Defining a strict release strategy for previews is hard. I think we should just act spontaneous on those but with clear communication.

there is no reason not to choose the latest [commit]

Agree

but we also need to update the README.MD to reflect the API changes

We definitely cannot do that. The readme is what the people follow if they want to get started using the library. We tell them to use the (latest stable) nuget version, so the readme has to be according to that version. We should document the previewed changes but I think we should do so in the GitHub release. Other places I could see that is in a separate section in the readme (but I don't really like that) or in a separate file. IMO the only thing we should mention in the readme is that there are previews available on nuget if the users want to try out the new features before they're released.

Joelius300 commented 4 years ago

As you might've seen, 2.0.0-preview1 has been released and published to NuGet.

Should we close this issue?

Cc @mariusmuntean

BorisGaliev commented 4 years ago

Thanks a lot for the new nuget package, today I tried to update it, but then I came across 2 issues that might be of interest to others.

Firstly, now I see an exception when building my solution: Error TS18003 Build:No inputs were found in config file 'C:/Users/.../.nuget/packages/chartjs.blazor/2.0.0-preview1/contentFiles/any/netstandard2.1/tsbuild/release/tsconfig.json'. Specified 'include' paths were '["../../wwwroot/ts/*.ts"]' and 'exclude' paths were '[]'.

Secondly, it's not clear how to display a chart where bar and line charts can be displayed at the same time, with newChart<BarConfig>it is not possible any more. Old ChartJsBarChart did allow to do that.

Any insights how to fix that?

Joelius300 commented 4 years ago

it's not clear how to display a chart where bar and line charts can be displayed at the same time, with new Chart<BarConfig> it is not possible any more. Old ChartJsBarChart did allow to do that.

Could you elaborate? The mixing aspect only changed slightly and it should even be easier now with the new datasets because you don't have to use wrappers (but there are still other issues). Can you show me the code that supposedly worked before but doesn't now?

Error TS18003 Build:No inputs were found in config file 'C:/Users/.../.nuget/packages/chartjs.blazor/2.0.0-preview1/contentFiles/any/netstandard2.1/tsbuild/release/tsconfig.json'. Specified 'include' paths were '["../../wwwroot/ts/*.ts"]' and 'exclude' paths were '[]'.

@mariusmuntean Could you please look into this? I'm assuming it has something to do with the recent update to the static assets (e.g. #130).

Ps. I'm hijacking this issue for general stuff regarding our preview releases if that's okay with you :) That's also the reason we should probably move the two issues you found into separate issues which we can tag with a preview-label or something like that.

BorisGaliev commented 4 years ago

@Joelius300 The issue with datasets occurs on config.Data.Datasets.Add(lineDataSet); Earlier it was possible to add LineDataset<ChartJs.Blazor.ChartJS.Common.Point> there on the BarDatasetCollection, in the last preRelease version only these methods are available : Add(IDataset<TimePoint> dataset), Add(IDataset<FloatingBarPoint> dataset), Add(IDataset<int> dataset), Add(IDataset<long> dataset), Add(IDataset<double> dataset) or do I am missing something?

@mariusmuntean With the preview Version of the nuget package a new tsbuild-Folder is added to solution, there is a link to tsconfig.json, related to C:\Users....nuget\packages\chartjs.blazor\2.0.0-preview1\contentFiles\any\netstandard2.1\tsbuild\release\tsconfig.json, I think this causes the issue

Joelius300 commented 4 years ago

Oh yea, I forgot I removed that. I disallowed adding point datasets to BarDatasetCollection because the bar chart doesn't understand those unless you specify a different type like line. The current solution with the dataset collections is nice when you exclude mixing scenarios like this but of course doesn't work for your use case. I will try to come up with a different solution for it.
Could you please open a new issue with a detailed explanation? I say "detailed".. you already did a good job in the last few comments but show us all the code, that's always helpful :)

alessandro-gargiulo commented 3 years ago

Any update regards the error TS18003 described by @BorisGaliev ? Does is it recommended to rollback to the latest stable version or is there any workaround (or fix)?

Joelius300 commented 3 years ago

Sorry for the late response. I'm not well versed with typescript compilation and don't have time to look into it currently. @mariusmuntean is the contributor to do it but anyone else could too if they submit a PR.

Ps. Once I have more time, I might start to work on 2.0 again but lately most of my efforts have been devoted to other projects, outside of ChartJs.Blazor.

fededim commented 3 years ago

Hi I am experiencing the same issue "No inputs were found in config file, @mariusmuntean any plan to look into it ?

kkokosa commented 3 years ago

It seems 2.0.0-preview1 NuGet package is in a pretty unusable state.

I needed to remove the line:

<files include="any/netstandard2.1/tsbuild/release/tsconfig.json" buildAction="Content" />

from c:\Users\XXX\.nuget\packages\chartjs.blazor\2.0.0-preview1\chartjs.blazor.nuspec to make it working.

BTW, example from the README is also outdated, it needs:

@using ChartJs.Blazor.Charts 
@using ChartJs.Blazor.Util
@using ChartJs.Blazor.ChartJS.PieChart
@using ChartJs.Blazor.ChartJS.Common.Properties

and PieDataset is not generic anymore.

I'm not contributig any fixes as I'm totally not-Typescript developer and I'm in this project for 15 minutes so you will probably know better how to use this knowledge :)

Joelius300 commented 3 years ago

It seems 2.0.0-preview1 NuGet package is in a pretty unusable state.

Indeed 😅 2.0 will be released very soon.

Ps. example from the README is also outdated is debatable because they are accurate for the latest ""stable"" release, not the preview.

Joelius300 commented 3 years ago

Hey, I just released version 2.0! Now in a more usable state than ever before :wink:

I encourage everyone to upgrade to 2.0. You can find a migration guide at the bottom of the release post.

This pre-release discussion was most relevant during the development of 2.0 so I'll close this issue now.