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

Template querySelector support #1320

Merged
merged 5 commits into from
Dec 17, 2019

Conversation

earnubs
Copy link
Contributor

@earnubs earnubs commented Oct 14, 2019

Handle selectors passed in the template option of the Vue instance (https://vuejs.org/v2/api/#template), currently test-utils gives an unfriendly error as the selector is passed as a template string:

browser.js:954 [Vue warn]: Error compiling template:

#foo

- Component template requires a root element, rather than just text.

Repro here: https://jsfiddle.net/earnubs/c5d2mea7/

@earnubs earnubs changed the title Template query selector support Template querySelector support Oct 14, 2019
@dobromir-hristov
Copy link
Contributor

A few notes:

  1. I have not thought about this case tbh, but I can see it being useful for people not using bundlers.
  2. lets remove the dist files, we can generate these on release.
  3. The test has a check, you can use the itDoNotRunIf.
  4. I am not sure how good of an idea it is to use document or window directly like that.

@lmiller1990
Copy link
Member

This is a great addition! Nice job.

As pointed out, try something like itDoNotRunIf(TEST_ENV === "node"). Full API for conditional-specs: https://github.com/eddyerburgh/conditional-specs.

Your suspicion about mutating window/document were correct @dobromir-hristov. For this reason, before and after each test, the window variable is reset, to keep things clean. See here.

I'm happy to approve this once

  • use itDoNotRunIf
  • remove dist files

Are you able to make these changes, @earnubs ?

@lmiller1990 lmiller1990 self-requested a review December 16, 2019 11:33
@@ -161,6 +161,26 @@ describeRunIf(process.env.TEST_ENV !== 'node', 'mount', () => {
expect(wrapper.html()).to.equal(`<div>foo</div>`)
})

it('compiles templates from querySelector', () => {
if (
!(navigator.userAgent.includes && navigator.userAgent.includes('node.js'))
Copy link
Member

Choose a reason for hiding this comment

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

see comments

@earnubs earnubs force-pushed the template-query-selector-support branch from 8a4c379 to e1c1b95 Compare December 16, 2019 12:05
@earnubs
Copy link
Contributor Author

earnubs commented Dec 16, 2019

Test is failing, I'll take a look at this tonight when I have a bit more time.

@earnubs earnubs requested a review from lmiller1990 December 16, 2019 15:57
@lmiller1990 lmiller1990 merged commit 0a72913 into vuejs:dev Dec 17, 2019
@vue-bot
Copy link

vue-bot commented Dec 17, 2019

Hey @earnubs, thank you for your time and effort spent on this PR, contributions like yours help make Vue better for everyone. Cheers! 💚

@earnubs earnubs deleted the template-query-selector-support branch December 17, 2019 09:06
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.

4 participants