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

Generated js files have SyntaxError when enabling source map in webpack config #1367

Closed
1 task done
jjyyxx opened this issue Feb 27, 2019 · 15 comments
Closed
1 task done
Labels
has PR Has a related PR type: bug Something isn't working

Comments

@jjyyxx
Copy link
Contributor

jjyyxx commented Feb 27, 2019

  • I confirm that this is a issue rather than a question.

Bug report

When enabling source map in the configureWebpack option, the generated js files have SyntaxErrors. Comment the devtool: "source-map" line solves the problem.

Version

1.0.0-alpha.40

Steps to reproduce

git clone https://github.com/jjyyxx/vuepress-1367-repro.git
cd vuepress-1367-repro
npm run build

What is expected?

Generate valid code.

What is actually happening?

Uncaught SyntaxError: Unexpected token } happens in app.xxx.js.

Other relevant information

  • Your OS: Windows 10 1809
  • Node.js version: v11.4.0
  • Browser version: Google Chrome v72.0.3626.119
  • Is this a global or local install? local
  • Which package manager did you use for the install? npm
  • Does this issue occur when all plugins are disabled? yes
@shigma shigma added the type: bug Something isn't working label Feb 27, 2019
@sinchang
Copy link
Contributor

it's working on my local macOS dev workspace.

@jjyyxx
Copy link
Contributor Author

jjyyxx commented Feb 28, 2019

@sinchang Thanks for your test! Could you upload your syntactically correct app.[hash].js file (rename to a txt)?

I have tested under Windows 10 and Ubuntu. Evaluating the app.[hash].js under both platforms led to

Uncaught SyntaxError: Unexpected token }

@sinchang
Copy link
Contributor

@jjyyxx
Copy link
Contributor Author

jjyyxx commented Feb 28, 2019

@sinchang Thanks for your quick reply. But when I try to evaluate your gist in the browser, the error

Uncaught SyntaxError: Unexpected token }

persists. Are you sure your gist could run without syntactical problems under macOS?

@shigma
Copy link
Collaborator

shigma commented Feb 28, 2019

@sinchang It seems that your gist begins with

//# sourceMappingURL=styles.bf997139.js.map!function(t){
//                                         ^ there should be a newline

This can explain the problem @jjyyxx encountered.

@sinchang
Copy link
Contributor

@jjyyxx sorry, my fault. build success but open in the browser has the same problem

@sinchang
Copy link
Contributor

@jjyyxx it's working when update config to

module.exports = {
  configureWebpack: () => {
    devtool: 'source-map'
  }
}

@jjyyxx
Copy link
Contributor Author

jjyyxx commented Feb 28, 2019

@sinchang You misused lambda.

In

() => {
    devtool: 'source-map'
    // "devtool:" is an unused label
    // 'source-map' is an unused string literal expression
}

It is pretty valid javascript but may not be what you what to achieve.
The correct way is

() => ({
  devtool: 'source-map'
})

which returns an object and fails as exprected.

@jjyyxx
Copy link
Contributor Author

jjyyxx commented Feb 28, 2019

I found that the function workaroundEmptyStyleChunk caused this problem.

async function workaroundEmptyStyleChunk () {
const styleChunk = stats.children[0].assets.find(a => {
return /styles\.\w{8}\.js$/.test(a.name)
})
if (!styleChunk) return
const styleChunkPath = path.resolve(outDir, styleChunk.name)
const styleChunkContent = await fs.readFile(styleChunkPath, 'utf-8')
await fs.remove(styleChunkPath)
// prepend it to app.js.
// this is necessary for the webpack runtime to work properly.
const appChunk = stats.children[0].assets.find(a => {
return /app\.\w{8}\.js$/.test(a.name)
})
const appChunkPath = path.resolve(outDir, appChunk.name)
const appChunkContent = await fs.readFile(appChunkPath, 'utf-8')
await fs.writeFile(appChunkPath, styleChunkContent + appChunkContent)
}

The hack was introduced to address this issue. This snippet prepends styleChunkContent to appChunkContent and writes it back to app.[hash].js. When building chunks without sourcemap, each chunk ends with a semicolon (always?). So the prepending operation generates valid code.

BUT, if sourcemap is configured to be generated, webpack will append a comment line to the end of each js chunk, which breaks the precondition of each chunk ends with a semicolon, leading to invalid code after prepending.

@jjyyxx
Copy link
Contributor Author

jjyyxx commented Feb 28, 2019

A simple workaround is inserting a line break when concatenating styleChunkContent and appChunkContent. While the SyntaxError will be resolved, another problem remains that the sourcemap for app.[hash].js will be invalidated because the whole chunk deviates from the original location by around 100 characters.

@yyx990803 The hack was introduced around a year ago, but its impact to sourcemap might not get considered yet. Could you offer some hints to solve this problem gracefully?

@shigma
Copy link
Collaborator

shigma commented Feb 28, 2019

Maybe we can make a simple judgment before using this workaround:

if (!config.devTool) await workaroundEmptyStyleChunk()

Since we usually don't need a sourcemap in a real production phase, I think this approach is understandable.

@yyx990803
Copy link
Member

@jjyyxx I think a more fundamental fix would be having the empty chunk issue fixed in webpack itself... thus getting rid of this hack altogether...

@jjyyxx
Copy link
Contributor Author

jjyyxx commented Feb 28, 2019

@shigma I tested the patch and it worked as expected. Is it possible to be added into the next version?

I don't think it's a breaking change. And it only introduces an if statement which can be removed together when the upstream issue fixed.

@shigma
Copy link
Collaborator

shigma commented Feb 28, 2019

@jjyyxx contribution welcome! 😆

@jjyyxx
Copy link
Contributor Author

jjyyxx commented Mar 1, 2019

pr created at #1378

@shigma shigma added the has PR Has a related PR label Mar 2, 2019
@ulivz ulivz closed this as completed in b53324d Mar 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has PR Has a related PR type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants