Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

feat(vue 3): update rollup-plugin-vue #952

Merged
merged 11 commits into from
Jun 10, 2021

Conversation

eunjae-lee
Copy link
Contributor

@eunjae-lee eunjae-lee commented Apr 14, 2021

This PR updates the build process into two, one for vue@2 and another for vue@3.

Although we do not use any specific Vue3-only APIs to render components, we'd better split the bundle outputs now for the future benefit.

Also, it's recommended to have different bundle outputs.

The order of the plugin has changed because of this issue.

References


How vue-demi works

It's a runtime dependency.

in Vue 2

import Vue from 'vue'

var isVue2 = true
var isVue3 = false
var Vue2 = Vue
var version = Vue.version

export {
  Vue,
  Vue2,
  isVue2,
  isVue3,
  version,
}

export const computed = undefined;
export const createApp = undefined;
export const createRef = undefined;
export const customRef = undefined;
...	// ↑ all the vue3-only APIs

in Vue 3

import * as Vue from 'vue'

var isVue2 = false
var isVue3 = true
var Vue2 = undefined

export * from 'vue'
export {
  Vue,
  Vue2,
  isVue2,
  isVue3,
}

So basically, the way we use this library is going to be

import { somethingVue3OnlyAPI, isVue3, Vue2 } from 'vue-demi';

if (isVue3) {
  somethingVue3OnlyAPI();
} else {
  Vue2.somethingElse();
}

@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 14, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 88a7223:

Sandbox Source
vue-instantsearch-e-commerce Configuration

@eunjae-lee eunjae-lee added this to the Vue 3 milestone May 28, 2021
Base automatically changed from feat/v3-compat-inject to feat/vue3-compat June 1, 2021 13:04
@eunjae-lee
Copy link
Contributor Author

eunjae-lee commented Jun 2, 2021

Okay, at this point, a very simple example works fine on both vue2 and vue3.

vue2-test.zip
vue3-test.zip
vue3-test-with-vite.zip

Unfortunately they don't work on CodeSandbox because in this PR of Vue InstantSearch, we're aliasing vue-demi like this:

"vue-demi": "npm:@eunjae-lee/[email protected]"

I don't think CodeSandbox is handling npm: prefix. I've sent an inquiry.

One caveat of the way this branch exports the bundles is,

import ... from 'vue-instantsearch'; // ← for vue2

import ... from 'vue-instantsearch/dist/vue3/es'; // ← for vue3

not sure how to shorten the import path for vue3, but we can do it in another PR.

@eunjae-lee eunjae-lee marked this pull request as ready for review June 8, 2021 08:46
@eunjae-lee eunjae-lee requested review from a team, tkrugg, shortcuts and Haroenv and removed request for a team and tkrugg June 8, 2021 08:47
src/util/getVueVersion.js Outdated Show resolved Hide resolved
const processEnv = conf => ({
// parenthesis to avoid syntax errors in places where {} is interpreted as a block
'process.env': `(${JSON.stringify(conf)})`,
});

const vuePlugin = process.env.VUE_VERSION === 'vue3' ? vueV3 : vueV2;
const outputDir =
process.env.VUE_VERSION === 'vue3' ? 'dist/vue3' : 'dist/vue2';
Copy link
Contributor

Choose a reason for hiding this comment

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

should the output dir for vue 2 stay the root?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I wanted to discuss with you about the output dirs.

With

  "main": "dist/vue2/cjs",
  "module": "dist/vue2/es",

that config, for the vue2 users, it will be the same. So I think it's okay to have it under dist/vue.

Where should we have vue3 bundles?

For now, I'm importing like this:

import ... from 'vue-instantsearch/dist/vue3/es';

package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@Haroenv
Copy link
Contributor

Haroenv commented Jun 8, 2021

can you remove the parts of vue-demi that we don't use in Vue InstantSearch, or do you think that isn't logical?

@eunjae-lee
Copy link
Contributor Author

can you remove the parts of vue-demi that we don't use in Vue InstantSearch, or do you think that isn't logical?

Yeah I already did it in my fork, including the removal of @vue/composition-api compatibility.

Do you see something specific that I missed?

@Haroenv
Copy link
Contributor

Haroenv commented Jun 8, 2021

@eunjae-lee
Copy link
Contributor Author

https://github.com/eunjae-lee/vue-demi/blob/master/lib/v2/index.esm.js#L17 is still there no?

deprecated usage also should go away: https://github.com/eunjae-lee/vue-demi/blob/master/lib/v2/index.d.ts#L6-L10

You're right. The second one should go away. However the first one should stay.

In Vue 2 mode,

/**VCA-EXPORTS**/
export * from '@vue/composition-api'
/**VCA-EXPORTS**/

becomes

/**VCA-EXPORTS**/
export const computed = undefined;
export const createApp = undefined;
export const createRef = undefined;
export const customRef = undefined;
export const defineAsyncComponent = undefined;
export const defineComponent = undefined;
export const del = undefined;
export const getCurrentInstance = undefined;
export const h = undefined;
export const inject = undefined;
export const isRaw = undefined;
export const isReactive = undefined;
export const isReadonly = undefined;
export const isRef = undefined;
export const markRaw = undefined;
export const nextTick = undefined;
export const onActivated = undefined;
export const onBeforeMount = undefined;
export const onBeforeUnmount = undefined;
export const onBeforeUpdate = undefined;
export const onDeactivated = undefined;
export const onErrorCaptured = undefined;
export const onMounted = undefined;
export const onServerPrefetch = undefined;
export const onUnmounted = undefined;
export const onUpdated = undefined;
export const provide = undefined;
export const proxyRefs = undefined;
export const reactive = undefined;
export const readonly = undefined;
export const ref = undefined;
export const set = undefined;
export const shallowReactive = undefined;
export const shallowReadonly = undefined;
export const shallowRef = undefined;
export const toRaw = undefined;
export const toRef = undefined;
export const toRefs = undefined;
export const triggerRef = undefined;
export const unref = undefined;
export const useCSSModule = undefined;
export const useCssModule = undefined;
export const warn = undefined;
export const watch = undefined;
export const watchEffect = undefined;
/**VCA-EXPORTS**/

They're necessary because when we do

import { isVue3, h } from 'vue-demi';


...

render(createElement) {
  return (isVue3 ? h : createElement)(...)
}

In vue 2, we still need h to be exported even if it's undefined.

@Haroenv
Copy link
Contributor

Haroenv commented Jun 9, 2021

I wonder if for our use case, we can manually do export const h = undefined

Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

this looks great!

Comment on lines -118 to +122
"path": "./dist/vue-instantsearch.js",
"path": "./dist/vue2/umd/index.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

changing existing file names could be a breaking change for when someone is forcing UMD at the moment, could we keep the existing names the same and for example only add a new folder for vue 3?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually no, let' have a clean dist and release this as a major version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I didn't realize we don't have jsdelivr or unpkg in the package.json at Vue InstantSearch. Yeah, the url of UMD will change. Let's go as a breaking change.

@eunjae-lee
Copy link
Contributor Author

I wonder if for our use case, we can manually do export const h = undefined

Definitely. I'll update vue-demi and apply it in another PR.

@eunjae-lee eunjae-lee merged commit b931ac0 into feat/vue3-compat Jun 10, 2021
@eunjae-lee eunjae-lee deleted the feat/v3-compat-rollup branch June 10, 2021 08:48
Haroenv pushed a commit to algolia/instantsearch that referenced this pull request Dec 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants