Skip to content

fix: compatible with webpack5 externals script #1774

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: compatible with webpack5 externals script #1774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/codegen/hotReload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ exports.genHotReloadCode = (id, functional, templateRequest) => {
/* hot reload */
if (module.hot) {
var api = require(${hotReloadAPIPath})
api.install(require('vue'))
api.install(Vue)
if (api.compatible) {
module.hot.accept()
if (!api.isRecorded('${id}')) {
Expand Down
6 changes: 5 additions & 1 deletion lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,11 @@ var component = normalizer(
}

if (needsHotReload) {
code += `\n` + genHotReloadCode(id, hasFunctional, templateRequest)
code = [
`import Vue from 'vue'`,
Copy link
Member

Choose a reason for hiding this comment

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

Does it have to be import? I'm not sure if it would break some CommonJS code.

Copy link
Author

Choose a reason for hiding this comment

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

if it would break some CommonJS code.

I'm not sure what 'some CommonJS code' means here

As far as I know, import in webpack can handle both ES Module and CommonJS

import Vue from 'vue/dist/vue.esm.js'
console.log(Vue.version) // works
import Vue from 'vue/dist/vue.common.js'
console.log(Vue.version) //works

but require is different

const Vue = require('vue/dist/vue.esm.js')
console.log(Vue.default.version)
const Vue = require('vue/dist/vue.common.js')
console.log(Vue.version)

So Vue = vue.__esModule ? vue.default : vue in vue-hot-reload-api is necessary for require('vue'). But it's redundant for import Vue from 'vue', since webpack will check __esModule in generated code and import Vue as expected.

Therefore, I think import Vue form 'vue' won't break codes below

var api = require(`path_to_vue-hot-reload-api`)// or import api from 'path_to_vue-hot-reload-api'
api.install(...)

Does it have to be import?

From the documentation, externalsType: 'script' works fine with import.

And it can work with require like example below. Promise fulfilled after the script is loaded.

require('vue').then(Vue => {
  
})

When using import Vue from 'vue' webpack will handle the Promise, I think it's because ESModule can be statically analyzed. Compared to require, webpack can do more for import at the compile stage.

I didn't see externalsType: script related discussions, maybe I can get more information from source code

Copy link
Member

Choose a reason for hiding this comment

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

Try this (in HelloWorld.vue):

import Vue from 'vue' // eslint-disable-line

module.exports = {
  name: 'HelloWorld',
  props: {
    msg: String
  }
}

vs

module.exports = {
  name: 'HelloWorld',
  props: {
    msg: String
  }
}

Copy link
Author

@fangbinwei fangbinwei Dec 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe the import statement added by vue-loader-v15 doesn't conflict with the sciprt in HelloWorld.vue?

image

some problem I find

  • webpack will generated some code to await the script tag resolve, it doesn't compatible with ie11 (Need to investigate)

Copy link
Member

Choose a reason for hiding this comment

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

Oh sorry, I forgot it is in another module rather than the script module…

code,
genHotReloadCode(id, hasFunctional, templateRequest)
].join('\n')
}

// Expose filename. This is used by the devtools and Vue runtime warnings.
Expand Down
14 changes: 14 additions & 0 deletions test/core.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,20 @@ test('basic', done => {
})
})

test('hot reload code run', done => {
mockBundleAndRun({
entry: 'basic.vue',
plugins: [
// bundle with hot-reload code
new (require('webpack').HotModuleReplacementPlugin)()
]
}, ({ jsdomError }) => {
// run code generated by 'genHotReloadCode' without error
expect(jsdomError).toBeUndefined()
done()
})
})

test('pre-processors', done => {
mockBundleAndRun({
entry: 'pre.vue',
Expand Down