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

Update deps #61

Merged
merged 10 commits into from
Nov 18, 2019
Merged

Update deps #61

merged 10 commits into from
Nov 18, 2019

Conversation

Dumluregn
Copy link
Collaborator

Updated deps and devdeps according to https://david-dm.org/ckeditor/ckeditor4-angular and https://david-dm.org/ckeditor/ckeditor4-angular?type=dev. TypeScript was only updated to version 3.5.3 as this is what Angular 8 allows (Angular 9 will require TS 3.6).

There were some issues to fix, primarily in unit tests, but they work fine now - the biggest change was rewriting test should create editor with CKEDITOR.${method} as if I understand this comment correctly it couldn't work the old way anymore.

e2e tests are currently broken as reported in #60.

Closes #32.
Closes #52.

Copy link
Contributor

@f1ames f1ames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, looks good 👍

One thing to check - I have update CI to run tests on IE11 too (not sure why we haven't done this earlier) and it was passing nicely (only one tests failing same way as for Edge) - https://travis-ci.org/ckeditor/ckeditor4-angular/builds/611462712#L305:

image

After rebasing this PR, it seems there is some issue on IE11, more tests are failing (or rather throws some errors) - https://travis-ci.org/ckeditor/ckeditor4-angular/builds/611467093#L414:

image

Could you investigate this case? Quick search gives these one threads on SO - https://stackoverflow.com/questions/46790493/pace-js-not-working-in-ie-11, but I'm not sure if it will be helpful.

@Dumluregn
Copy link
Collaborator Author

The reason for failing test for IE11 were missing polyfills. I didn't bother to add them before as they were unnecessary for CKEditor, but turned out to be relevant here - we need polyfills for both tests using console and when ensuring that divarea plugin is loaded. Deleting them earlier was pretty reckless, sorry 😞

I wanted to mention one more thing. I see that updating ng-packgr in CKEditor5-angular resulted in breaking production builds. I tried to reproduce this error in our repo and it seems to work fine, but still please check it too before merging.

@f1ames f1ames self-assigned this Nov 18, 2019
@f1ames
Copy link
Contributor

f1ames commented Nov 18, 2019

Rebased onto latest master.

Copy link
Contributor

@f1ames f1ames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see now CI is green 💚 (pushed small fix with increased timeout since one tests is randomly failing on Edge/IE11)

I have checked with Angular 5 project and it looks good. Anyway, good catch with this issue 👍 Btw. there is a nice SO answer how to test with locally build package - https://stackoverflow.com/a/53794086.

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.

Update dependencies Update karma dependency
2 participants