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 V13 support. #133

Closed
Mekacher-Anis opened this issue Nov 10, 2021 · 13 comments
Closed

Angular V13 support. #133

Mekacher-Anis opened this issue Nov 10, 2021 · 13 comments

Comments

@Mekacher-Anis
Copy link

Mekacher-Anis commented Nov 10, 2021

Angular v13 has been released this week.
Any plans to publish a version that works with v13 ?
I suppose since the angular team is pushing to dropping View Engine, I think this would be a good time to switch to using native Ivy (no ngcc). See this issue angular/angular-cli#16980 .
I don't have any experience developing or building angular libraries, but I'm willing to get into it and switch this library to using the Ivy rendering engine. Any Docs or guidelines will be appreciated.

@renemadsen
Copy link

Any changes to this?

@danielmoncada
Copy link
Owner

danielmoncada commented Nov 10, 2021

not yet.. I'll work on getting it out today.

edit: @renemadsen I see that you forked this to add 13 support. would you mind creating a PR for this?

@renemadsen
Copy link

renemadsen commented Nov 11, 2021

@danielmoncada the fork was just an attempt to see, what is missing for updating to angular 13. But since I don't really know what the code is trying to do here, it's difficult to fix it:
https://github.com/microting/date-time-picker/runs/4169108246?check_suite_focus=true#step:6:15

    private watchStateChanges(): void {
        this.stateChanges.unsubscribe();

        const inputDisabled = this.dtPicker && this.dtPicker.dtInput ?
            this.dtPicker.dtInput.disabledChange : observableOf();

        const pickerDisabled = this.dtPicker ?
            this.dtPicker.disabledChange : observableOf();

        this.stateChanges = merge(pickerDisabled, inputDisabled)
            .subscribe(() => {
                this.changeDetector.markForCheck();
            });
    }
Argument of type 'Observable<never> | EventEmitter<boolean>' is not assignable to parameter of type 'number'.

If you can tell what is the solution for this?

At first glance, the above error, is the only thing stopping the code from working with angular 13, so far at least :-)

@marleypowell
Copy link

@danielmoncada the fork was just an attempt to see, what is missing for updating to angular 13. But since I don't really know what the code is trying to do here, it's difficult to fix it: https://github.com/microting/date-time-picker/runs/4169108246?check_suite_focus=true#step:6:15

    private watchStateChanges(): void {
        this.stateChanges.unsubscribe();

        const inputDisabled = this.dtPicker && this.dtPicker.dtInput ?
            this.dtPicker.dtInput.disabledChange : observableOf();

        const pickerDisabled = this.dtPicker ?
            this.dtPicker.disabledChange : observableOf();

        this.stateChanges = merge(pickerDisabled, inputDisabled)
            .subscribe(() => {
                this.changeDetector.markForCheck();
            });
    }
Argument of type 'Observable<never> | EventEmitter<boolean>' is not assignable to parameter of type 'number'.

If you can tell what is the solution for this?

At first glance, the above error, is the only thing stopping the code from working with angular 13, so far at least :-)

Does it work if you use a different overload of the merge operator? Wrap the observables in an array:

this.stateChanges = merge([pickerDisabled, inputDisabled]).subscribe(() => {
  this.changeDetector.markForCheck();
});

@renemadsen
Copy link

renemadsen commented Nov 12, 2021

Now it can build, but the tests are blowing up.

I don't know karma or how these tests are written, so I don't know how to debug them.
https://github.com/microting/date-time-picker/runs/4193651309?check_suite_focus=true#step:7:1

Any suggestions @danielmoncada ?

@harvanchik
Copy link

Any update on Angular 13 support?

@danielmoncada
Copy link
Owner

danielmoncada commented Dec 5, 2021

Updated via #137

Can you guys @Mekacher-Anis @renemadsen @marleypowell @harvanchik make sure it works on your project before I take it out of alpha? https://www.npmjs.com/package/@danielmoncada/angular-datetime-picker/v/13.0.0-alpha.0

@danielmoncada
Copy link
Owner

Now it can build, but the tests are blowing up.

I don't know karma or how these tests are written, so I don't know how to debug them. https://github.com/microting/date-time-picker/runs/4193651309?check_suite_focus=true#step:7:1

Any suggestions @danielmoncada ?

Your error indicates it cant find the coverage reporter when it runs.. a bit weird, as I set up the same workflow here and tests are running fine: https://github.com/danielmoncada/date-time-picker/actions

@marleypowell
Copy link

Updated via #137

Can you guys @Mekacher-Anis @renemadsen @marleypowell @harvanchik make sure it works on your project before I take it out of alpha? https://www.npmjs.com/package/@danielmoncada/angular-datetime-picker/v/13.0.0-alpha.0

Seems to be working for me with no errors. Nice work 🙂

@renemadsen
Copy link

renemadsen commented Dec 6, 2021

@danielmoncada seems to work. And now I see the difference, I changed the package.json in the root project and not inside the projects folder.

That is what making my repository break.

But why do we have it like that?

@danielmoncada
Copy link
Owner

@danielmoncada seems to work. And now I see the difference, I changed the package.json in the root project and not inside the projects folder.

That is what making my repository break.

But why do we have it like that?

this is how it was done by the original author. it's separated. the root project is for the "test" app that's coupled with the date/time picker library. any dependency changes on the library itself has to be done with the package.json in the 'projects' folder.

@renemadsen
Copy link

this is how it was done by the original author. it's separated. the root project is for the "test" app that's coupled with the date/time picker library. any dependency changes on the library itself has to be done with the package.json in the 'projects' folder.

Makes sense.

I'll try to upgrade it to angular 13, so the project is not so far behind.

@danielmoncada
Copy link
Owner

closing issue, as this was geared towards ng 13 support. since it now works, I'll create a new issue to upgrade it to ng 13.

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

No branches or pull requests

5 participants