support multi-entrypoint bundling(webpack) #144
Conversation
|
Awesome! Thanks for contributing. @drwpow want to review this one? |
|
It still only work for SPA, but can support the case that have more than one local js file. |
packages/plugin-webpack/plugin.js
Outdated
| const jsOutputPattern = args.outputPattern.js || "js/bundle-[hash].js"; | ||
| const cssOutputPattern = args.outputPattern.css || "css/style-[hash].css"; | ||
| const jsOutputPattern = args.outputPattern.js || "js/[name].[hash].js"; | ||
| const cssOutputPattern = args.outputPattern.css || "css/[name].[hash].css"; |
There was a problem hiding this comment.
🤔 should we use [contenthash] here instead of [hash] (hash based on file contents)? If you don’t have an opinion, we can leave it for now.
There was a problem hiding this comment.
Agree.
[contenthash] should be better than [hash]. I leave it unmodified just because it use [hash] before.
I will make an update.
| //Find all local script, use it as the entrypoint | ||
| const scripts = Array.from(dom.window.document.querySelectorAll("script")) | ||
| .filter(el => el.type.trim().toLowerCase() === "module") | ||
| .filter(el => !/^[a-zA-Z]+:\/\//.test(el.src)); |
There was a problem hiding this comment.
This is easier for me to follow. Nice! 👍
packages/plugin-webpack/plugin.js
Outdated
| const parsedPath = path.parse(src); | ||
| const name = parsedPath.name; | ||
| if (entries.name !== undefined) { | ||
| throw new Error(`Duplictate script with name ${name}`); |
There was a problem hiding this comment.
I like the approach you did here of using the JS filename as the entry name. While collisions can probably be a little confusing here for users (“Duplicate script? Where? How do I fix that?”), I think this error message is as good as it can be. I’d love to add a little more info on how this works, maybe on https://www.snowpack.dev/plugins. Because this is an “official” plugin
| const jsFile = assetFiles.find((d) => d.endsWith(".js")); | ||
| const cssFile = assetFiles.find((d) => d.endsWith(".css")); | ||
| script.removeAttribute("type"); | ||
| script.src = path.posix.join(baseUrl, jsFile); |
There was a problem hiding this comment.
Not related to this PR, just thinking out loud: I think this path.posix.join will probably print backslashes on Windows, which is not what we want. We should probably replace this with a web-friendly, forward-slash-only method in the future.
There was a problem hiding this comment.
path.posix.join will force path to join with a /, both in windows and linux.
|
@W1U02 the spelling error is the only change you might want to make; otherwise it’s amazing. Ready to merge whenever you’re ready! |
change [hash] to [contenthash] fix typo
|
@drwpow thanks for the comments. I've make an update. change |
|
Awesome work, @W1U02! 🙏 |
|
Can someone help me with Where should i include the buildOptions object? I tried in a bunch of places but none worked |
|
@haikyuu |
|
Thank you @drwpow |
buildOptions