Conversation
✅ Deploy Preview for vue-test-utils-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
src/index.ts
Outdated
| import { flushPromises } from './utils/flushPromises' | ||
| import { enableAutoUnmount, disableAutoUnmount } from './utils/autoUnmount' | ||
|
|
||
| // is __SSR__ avaialble? If so, preferable to use that? |
There was a problem hiding this comment.
In Vue Core SSR defined in rollup config https://github.com/vuejs/core/blob/da4a4fb5e8eee3c6d31f24ebd79a9d0feca56cb2/rollup.config.js#L148
There was a problem hiding this comment.
I don't know what's best. Maybe a variable defined in the rollup config would make more sense.
@lmiller1990 I think you were the one who set up the build config. Do you have an opinion on this?
There was a problem hiding this comment.
I'd expect we want to mirror what core does where possible!
There was a problem hiding this comment.
Either way, if it works, and we've got a test, happy to move forward with this. Did you want to make a change @Sandbagger? Any opinion @cexbrayat ?
There was a problem hiding this comment.
Vue core uses a build variable, so I think it would be better to mirror that if @Sandbagger feels like trying this. Otherwise I'm OK with merging this (once the comment is removed)
There was a problem hiding this comment.
Thanks chaps. I'll give rollup config variable a go.
src/index.ts
Outdated
| import { flushPromises } from './utils/flushPromises' | ||
| import { enableAutoUnmount, disableAutoUnmount } from './utils/autoUnmount' | ||
|
|
||
| // is __SSR__ avaialble? If so, preferable to use that? |
There was a problem hiding this comment.
I'd expect we want to mirror what core does where possible!
src/index.ts
Outdated
| import { flushPromises } from './utils/flushPromises' | ||
| import { enableAutoUnmount, disableAutoUnmount } from './utils/autoUnmount' | ||
|
|
||
| // is __SSR__ avaialble? If so, preferable to use that? |
There was a problem hiding this comment.
Either way, if it works, and we've got a test, happy to move forward with this. Did you want to make a change @Sandbagger? Any opinion @cexbrayat ?
|
@cexbrayat @cexbrayat I need to put a little bit more thought into using a build variable and how best to test it. I will spend some time over the weekend to do this with a view to opening another MR. In the meantime would you both be happy if we get this in (I've removed the comment)? |
|
@Sandbagger You can take your time and polish the PR no worries. I prefer to merge the final one 👍 |
3 participants
Work in progress
#2261
Cause:
Fix
Notes