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

T/51: Used config.initialData instead of innerHTML to set the initial state of the editor #52

Merged
merged 3 commits into from
Jun 14, 2019

Conversation

oleq
Copy link
Member

@oleq oleq commented Apr 24, 2019

Suggested merge commit message (convention)

Internal: Used config.initialData instead of innerHTML to set the initial state of the editor. Closes #51.


Additional information

Nuke the node_modules before running tests. The newer build-classic is required to support initialData.

@oleq oleq requested a review from ma2ciek April 24, 2019 09:28
@coveralls
Copy link

coveralls commented Apr 24, 2019

Pull Request Test Coverage Report for Build 85

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 83: 0.0%
Covered Lines: 30
Relevant Lines: 30

💛 - Coveralls

@ma2ciek
Copy link
Contributor

ma2ciek commented May 9, 2019

I see that now there're such warnings on CI:

ERROR: '[vue-test-utils]: Global error handler detected (Vue.config.errorHandler). 
Vue Test Utils sets a custom error handler to throw errors thrown by instances. If you want this behavior in your tests, you must remove the global error handler.'

Are they correct?
Can we somehow remove them?

@oleq
Copy link
Member Author

oleq commented Jun 13, 2019

They are incorrect. Or not very accurate. Because tests use Vue.nextTick, things are asynchronous and done() is used in places like this https:/ckeditor/ckeditor5-vue/blob/t/51/tests/ckeditor.js#L50 in the past.

I added Vue.config.errorHandler = done; in all tests as recommended in the official guide to improve readability of errors.

Before:
image

After:
image

It helps a lot but for some reason, vue-tests-utils displays an error (which kinda contradicts their documentation...). Maybe this is because vue-tests-utils is still beta (?). Anyway, it's not changing anything but DX so we can either remove the Vue.config.errorHandler = done; line and live with ugly Mocha errors or just live with quirky errors from vue-tests-utils.

@oleq
Copy link
Member Author

oleq commented Jun 14, 2019

What about the PR itself? Does it make sense?

@ma2ciek
Copy link
Contributor

ma2ciek commented Jun 14, 2019

Overall the PR is good and the change makes sense, I'm merging it.

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.

Change way of setting data
3 participants