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

Sunsetting webpack-serve #1195

Merged
merged 12 commits into from
Jan 27, 2019
Merged

Sunsetting webpack-serve #1195

merged 12 commits into from
Jan 27, 2019

Conversation

ulivz
Copy link
Member

@ulivz ulivz commented Jan 15, 2019

TL;DR: Any technology has a suitable application scenario. For vuepress, express is actually more suitable than koa.

Why do we abandon webpack-serve and choose webpack-dev-server?

There are several main reasons for us to embrace webpack-dev-server.

  1. The underlying shelf that we used to implement dev-server and HMR is webpack-serve which is based on koa, not long ago the maintainer announced the deprecation of webpack-serve and recommended us to choose webpack-dev-server that is based on express.

  2. Due to its more succinct usage, webpack-dev-server is used by many popular build tools, e.g. vue-cli, poi etc. combining with following 3rd point, you'll find that webpack-dev-server will help us to reduce the cost of switching between similar projects and writing plugins.

  3. In the 1.0 release plan, we supported a plugin API enhanceDevServer which allows us to tweak the instance of underlying koa app. although koa‘s nonlinear architectural model looks cooler, but the reality is that the ecology of express is better, as well as the cost of writing middleware is lower than koa.

    e.g. in koa, if you want to use connect-history-api-fallback, you'll need koa-connect for which you needn't care in express.

Migration

For 0.x users, there is no any effect since we didn't expose API to modify it.
For 1.x users whose version is lower than 1.0.0-alpha.32:

  1. You should use beforeDevServer(e.g. before in webpack-dev-server) to replace enhanceDevServer;
  2. You can use afterDevServer(e.g. after in webpack-dev-server) to execute custom middleware after all other middleware internally within the server.

e.g. writing a plugin to create sub public directory images:

  1. In the past (>= 1.0.0-alpha.0 && <= 1.0.0-alpha.32):
const { fs, path } = require('@vuepress/shared-utils')
const mount = require('koa-mount')
const serveStatic = require('koa-static')

module.exports = (options, context) => {
  const imagesAssetsPath = path.resolve(context.sourceDir, '.vuepress/images')

  return {
      // For development
      enhanceDevServer (app) {
        app.use(mount(path.join(context.base, 'images'), serveStatic(imagesAssetsPath)))
      },

      // For production
      async generated () {
        await fs.copy(imagesAssetsPath, path.resolve(context.outDir, 'images'))
      }
  }
}
  1. With this PR (>= 1.0.0-alpha.33):
const { fs, path } = require('@vuepress/shared-utils')
const express = require('express')

module.exports = (options, context) => {
  const imagesAssetsPath = path.resolve(context.sourceDir, '.vuepress/images')

  return {
    // For development
    beforeDevServer (app) {
      app.use('/images', express.static(imagesAssetsPath))
    },

    // For production
    async generated () {
      await fs.copy(imagesAssetsPath, path.resolve(context.outDir, 'images'))
    }
  }
}

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes
  • No

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)

You have tested in the following browsers: (Providing a detailed version will be better.)

  • Chrome
  • Firefox
  • Safari
  • Edge
  • IE

@ulivz ulivz changed the title feat($core): migrate to webpack-dev-server [WIP] feat($core): lerverage webpack-dev-server Jan 26, 2019
Copy link
Member Author

@ulivz ulivz left a comment

Choose a reason for hiding this comment

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

Just made some notes for this pull request.

@@ -105,52 +100,49 @@ module.exports = async function dev (sourceDir, cliOptions = {}) {
config = applyUserWebpackConfig(userConfig, config, false /* isServer */)
}

const serverConfig = {
disableHostCheck: true,
compress: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

✅Used to bnable gzip compression for everything served.

@@ -105,52 +100,49 @@ module.exports = async function dev (sourceDir, cliOptions = {}) {
config = applyUserWebpackConfig(userConfig, config, false /* isServer */)
}

const serverConfig = {
disableHostCheck: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

✅Used to bypasses host checking.

const serverConfig = {
disableHostCheck: true,
compress: true,
clientLogLevel: 'none',
Copy link
Member Author

Choose a reason for hiding this comment

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

❌clientLogLevel should follow the past configuration "error"

disableHostCheck: true,
compress: true,
clientLogLevel: 'none',
hot: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

✅Enable webpack's Hot Module Replacement feature.

compress: true,
clientLogLevel: 'none',
hot: true,
quiet: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

With devServer.quiet enabled, nothing except the initial startup information will be written to the console. This also means that errors or warnings from webpack are not visible.

❌This option shouldn't be set to true.

]
},
overlay: false,
host,
Copy link
Member Author

Choose a reason for hiding this comment

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

},
overlay: false,
host,
contentBase: path.resolve(sourceDir, '.vuepress/public')
Copy link
Member Author

Choose a reason for hiding this comment

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

✅contentBase was implemented by express.static.

// ctx.pluginAPI.options.enhanceDevServer.syncApply(app)
// app.use('/', express.static(path.resolve(sourceDir, '.vuepress/public')))
// }
}
Copy link
Member Author

Choose a reason for hiding this comment

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

❌We can open a Plugin Option API called beforeDevServer and afterDevServer

// A plugin
module.exports = {
  beforeDevServer (app, server) {

  },
  afterDevServer (app, server) {

  }
}

const compiler = webpack(config)
const server = new WebpackDevServer(compiler, serverConfig)
Copy link
Member Author

Choose a reason for hiding this comment

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

❌We should create a Node.js API for dev so that plugin developers can do more things.

e.g. custom a command to export current VuePress site to a PDF.

const { dev } = require('vuepresss')

module.exports = {
  extendCli (cli) {
    cli
      .command('export [targetDir]', 'export current vuepress site to a PDF')
      .action(async (dir = '.') => {
        const { port, host, server } = await dev.prepare()
        server.listen(port, host, err => {
          if (err) {
            console.log(err)
          }
        })
        // do somthing, such as launch browser to export PDF.
        await generatePDF()

        server.close()
      })
  }
}

const userPublic = path.resolve(sourceDir, '.vuepress/public')

// enable range request
app.use(range)
Copy link
Member Author

Choose a reason for hiding this comment

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

✅Express enable range request in development. so removing it will not break the fix of #555.

Copy link
Member Author

@ulivz ulivz left a comment

Choose a reason for hiding this comment

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

Just made some notes for this pull request.

@ulivz ulivz changed the title feat($core): lerverage webpack-dev-server Sunsetting webpack-serve Jan 27, 2019
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.

2 participants