Open la-we opened 1 year ago
Thanks you for your feedback @lweberprb.
It's true that it wasn't particularly necessary. I did it to comply with the eslint rule no-explicit-any
. Since this rule is not uniformly respected in the library anyway, I'm going to publish a corrective patch (14.0.3
) later this evening to fix this breaking change, which doesn't add anything to what I originally released version 14 for.
Thank you very much for your quick response!
A neat change could be a generic getData<T> => _data as T;
to directly have a cast built into the API in addition to a deprecated getData(): any
method. But this is obviously up to you and/or the community to decide.
Anyhow, thanks for your awesome work!
@lweberprb Great idea! Btw I won't forget you, I'm taking advantage of the patch to make a few additional fixes. 👌
Hi @maximelafarie I see that https://github.com/maximelafarie/ngx-smart-modal/pull/380 got merged in, and seems to have created a 14.0.2 release in github, but the actions output for semantic release seems to think a release should not have been created. Is that in error? https://github.com/maximelafarie/ngx-smart-modal/actions/runs/6791148890/job/18462433573#step:6:42
Also as there was already a 14.0.2 release in npm it seems to be causing some confusion.
Hi @trdyer, yes, something strange happened with the semantic-release
command, despite the fact that it should run in "dry-run" mode.
I'll make a manual release right now.
Once again, thanks a million for your patience, folks!
I've just released v14.0.3, which includes the @AntLer-24rus fixes. Don't hesitate to give me feedback.
Hi @maximelafarie, thank you so much for all your amazing work. With v14.0.3, I still see that getData(): unknown and this is causing errors in my build error TS2339: Property 'data' does not exist on type 'unknown'.
. Are there any plans to add a type instead of unknown?
Hi @jenniferchau, thank you very much for the kind words. Absolutely, I haven't fixed this for a long time and it's a problem for a lot of people. I'm going to fix it!
Is there any progress on changing it back to any
? It is hindering the upgrade path for my organisation.
Hi, there,
sorry, I got married recently and have had a lot going on at the moment which (once again) has made it very difficult for me to refocus.
I have two solutions in mind:
a simple rollback:
public getData(): any {
this.assignComponentDataToModalData(this._componentRef);
return this._data;
}
an improvement that leaves you free to define the expected type:
public getData<T>(): T {
this.assignComponentDataToModalData(this._componentRef);
return this._data as T;
}
Tell me what you think is best (in the general interest). I'll include it in the batch of fixes I want to roll out soon. (I think that free typing is the most interesting 😄)
Congratulations! Really happy for you!
From my perspective the 2nd solution would be the best, as it soemwhat enforces the user to explicity specify a type that is expected to be returned from the getData
function.
With the newest version, the return type of
getData()
changed fromany
tounknown
. This should be included in the list of breaking changes. Why was this change necessary in the first place?See: code