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

NPM Quill breaks Meteor Testing With Mocha #1612

Closed
SevenZark opened this issue Jul 31, 2017 · 16 comments
Closed

NPM Quill breaks Meteor Testing With Mocha #1612

SevenZark opened this issue Jul 31, 2017 · 16 comments

Comments

@SevenZark
Copy link

SevenZark commented Jul 31, 2017

I understand this may be outside of the scope of what Quill wishes to support, but it does rule out Quill for those using Meteor, a very popular app framework.

Steps for Reproduction

  1. In any Meteor app file, include Quill like so: import Quill from 'quill'
  2. On command line, run meteor test: meteor test --driver-package practicalmeteor:mocha

Expected behavior: Tests run normally

Actual behavior: Get the following error:

ReferenceError: document is not defined
at Object.defineProperty.value (/Users/[redacted]/Source/[redacted]/node_modules/quill/dist/quill.js:6795:12)
at webpack_require (/Users/[redacted]/Source/[redacted]/node_modules/quill/dist/quill.js:36:30)
at Object.defineProperty.value (/Users/[redacted]/Source/[redacted]/node_modules/quill/dist/quill.js:1109:1)
at webpack_require (/Users/[redacted]/Source/[redacted]/node_modules/quill/dist/quill.js:36:30)
at Object.defineProperty.value (/Users/[redacted]/Source/[redacted]/node_modules/quill/dist/quill.js:5011:14)
at webpack_require (/Users/[redacted]/Source/[redacted]/node_modules/quill/dist/quill.js:36:30)
at Object.defineProperty.value (/Users/[redacted]/Source/[redacted]/node_modules/quill/dist/quill.js:9554:13)
at webpack_require (/Users/[redacted]/Source/[redacted]/node_modules/quill/dist/quill.js:36:30)
at Object. (/Users/[redacted]/Source/[redacted]/node_modules/quill/dist/quill.js:11266:18)
at webpack_require (/Users/[redacted]/Source/[redacted]/node_modules/quill/dist/quill.js:36:30)
Exited with code: 1

Platforms: Meteor, Mocha, mac CLI (can't test if it happens in Windows)

Quill Version: 1.3.0

@jhchen
Copy link
Member

jhchen commented Jul 31, 2017

What have you tried?

@SevenZark
Copy link
Author

SevenZark commented Jul 31, 2017

I really don't know what I could try, since just importing Quill in a template causes the test to break. The only way I could fix it was to not import Quill, i.e. not use Quill.

@SevenZark
Copy link
Author

SevenZark commented Jul 31, 2017

It looks like simply having the Quill library present makes it want to immediately render something in the DOM ('document'). This won't work in a unit test since there is no actual DOM.

@SevenZark
Copy link
Author

I should add, it does not matter what part of the code I am testing. In fact, I had no test code that dealt with the page where I wanted to use Quill. Just importing Quill in an app file caused the test process to bail and show the error.

@abramz
Copy link

abramz commented Aug 4, 2017

Does mocha create a browser environment?

@SevenZark
Copy link
Author

Not if you don't request one. Trouble is, I get this error when NOT loading templates in mocha; just loading server modules to test. The mere fact that the files exist in the project causes this error.

@abramz
Copy link

abramz commented Aug 6, 2017

You are importing Quill though, @SevenZark, and that requires the DOM. As you can see here, core/polyfill is imported, which runs on import & requires document.

@jhchen
Copy link
Member

jhchen commented Aug 7, 2017

Yes Quill needs the DOM to work. There are too many places where Quill references document that it would be silly to guard against for an environment (server node) it is not built for.

@jhchen jhchen closed this as completed Aug 7, 2017
@SevenZark
Copy link
Author

SevenZark commented Aug 7, 2017

I'm sorry, this is a bit hard to describe, I guess. Of course I understand Quill needs the DOM to work when Quill is invoked, and in files in which it is called. Please try to understand, maybe don't assume I know nothing at all even if I don't know as much as you or much about the inner complexities of the libraries in question.

What I am trying to tell you is that I am NOT importing Quill in any of the modules I am TESTING. They are NOT loaded in my test script, period. Nonetheless, the test fails right on the command line when trying to invoke it, complaining about the error I mentioned. The fact that Quill appears in an import statement in a file at all in the source code causes this error to happen even when I AM NOT LOADING THAT FILE IN THE TEST. It seems that something about Quill loads very, very eagerly, in some way...? Whatever is going on, the fact is, if I wish to use best practices and test, I cannot have an import of Quill appear anywhere in any file in my code base, whether or not I am actually writing test script against that file.

This is not a problem with any other UI library I use, including React. I guess React must be doing something pretty "silly," then? And all those other libraries are just "silly" too? Quill is very distinguished in being the only truly non-silly library I have tried to use. ;)

As I stated in the original post, I fully realize this may be outside the scope of what Quill wishes to accommodate. That's fine. It's an external, sorta "dependency." But I thought it was surprising and thought it was worth mentioning since it completely rules out Quill for use in any Meteor application that uses the recommended test engine for Meteor. I thought you might want to know that. I don't have any expectation that it would be a high priority or anything. But please don't just call the concern "silly."

@abramz
Copy link

abramz commented Aug 7, 2017

Of course I understand Quill needs the DOM to work when Quill is invoked, and in files in which it is called.

@SevenZark, Quill requires the DOM when it is imported, not when it is invoked. I think this may be what you are missing. The very fact that you import Quill, which you say you did in repro step 1, will cause breakage in an environment that does not provide the DOM. If you review the files in my previous reply, you will see that core/polyfill runs on import.

@SevenZark
Copy link
Author

SevenZark commented Aug 7, 2017

Quill requires the DOM when it is imported, not when it is invoked. I think this may be what you are missing.

@abramz Yeah, I do understand that. What I'm trying to tell you is that, for that reason, I DON'T load any of the files which import quill, in my test scripts. For some reason mocha throws up over it anyway. Do you get me? I'm not loading any client side modules in my server tests in the first place, because I'm not testing the client. For some strange reason they fail anyway with errors from Quill complaining that there is no doc object. It's a surprising problem, and maybe that's why it's hard for me to explain it(?)

It could very easily be some strange, esoteric mocha bug that only Quill has triggered for me. I have no idea. I mean, it doesn't make any sense that mocha would be loading modules I'm not telling it to. A testing library like mocha doesn't normally eagerly load any resources you don't tell it to, which is why you have to stub and mock things sometimes. It would defeat the whole purpose of, e.g., unit testing.

Sorry I can't provide more insight about it. And like I said, I understand if it isn't something the Quill team can address. I might try asking about it in the Mocha world as well.

@SevenZark
Copy link
Author

I can see 'invoke' was poor wording on my part. Let me attempt to clarify.

  • I DO have files in my project which import Quill
  • However, I DO NOT include those files in the Mocha tests
  • I get errors from Quill about a missing document object anyway

Regarding this strange situation:

  • I get that supporting Mocha isn't necessarily a priority for Quill since it's not a Quill dependency, and,
  • I get that it could be some esoteric Mocha bug too, so I may inquire in that world as well

@abramz
Copy link

abramz commented Aug 7, 2017

How would an error get thrown from Quill if it were not imported?

@SevenZark
Copy link
Author

Exactly! No idea, but it is.

@abramz
Copy link

abramz commented Aug 7, 2017

Exactly, so, let's use Occam's razor and say that QuillJS is getting imported somewhere.

What would the parameters be of a scenario where this is not the case?

@SevenZark
Copy link
Author

Darn you and your logic! ;) I get your point. The problem is either something in how I'm configuring my test, or something weird that Mocha is doing.

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

3 participants