esbuild: "naive" finding the the outputPath in manifest buggy, if node_modules imports are used in scss

[zealot128]

Bridgetown Version: 1.1.0

To Reproduce:

• New Bridgetown site
• `yarn add sass, Add bridgetown Esbuild
• and add any 3rd party css (e.g. yarn add mvp.css or yarn add @fontsource/some-gfont)
• run bridgetown dev

Current behavior

esbuild: There was an error parsing your css file. Please check your css file for any errors

binding.irb into bridgetown-core-1.1.0/lib/bridgetown-core/utils.rb#parse_esbuild_manifest_file reveals that index.css does got the manifest name "node_modules/mvp.css/mvp.css" => "index.12da9828.css", but BT expects an "frontend/index.css" as a key instead.

Fix/Workaround:

I was suspecting the manifest generation in config/esbuild.defaults.js:

esbuild(?sass?) decided, to put the node_modules first console.log(result.metafile.outputs)

So, changing, the "naive" implementation to something more meaningful (find something which includes "index") worked: ```diff diff --git a/config/esbuild.defaults.js b/config/esbuild.defaults.js index 0295d4e..59421e2 100644 --- a/config/esbuild.defaults.js +++ b/config/esbuild.defaults.js @@ -239,7 +239,7 @@ const bridgetownPreset = (outputFolder) => ({ entrypoints.push([outputPath, fileSize(key)]) } else if (inputs.length > 0) { // Naive implementation, we'll just grab the first input and hope it's accurate - manifest[stripPrefix(inputs[0])] = outputPath + manifest[stripPrefix(inputs.find(e => e.includes('index')) || inputs[0])] = outputPath } } ``` **EDIT** That workaround does not work, still... Trying something else...

Update: that is the correct fix:

diff --git a/config/esbuild.defaults.js b/config/esbuild.defaults.js
index 0295d4e..ff8e053 100644
--- a/config/esbuild.defaults.js
+++ b/config/esbuild.defaults.js
@@ -233,13 +233,15 @@ const bridgetownPreset = (outputFolder) => ({
           // We have an entrypoint!
           manifest[stripPrefix(value.entryPoint)] = outputPath
           entrypoints.push([outputPath, fileSize(key)])
-        } else if (key.match(/index(\.js)?\.[^-.]*\.css/) && inputs.find(item => item.match(/\.(s?css|sass)$/))) {
+        } else if (key.match(/index(\.js)?\.[^-.]*\.css/) && inputs.find(item => item.match(/frontend.*\.(s?css|sass)$/))) {
+          const input = inputs.find(item => item.match(/frontend.*\.(s?css|sass)$/))
           // Special treatment for index.css
-          manifest[stripPrefix(inputs.find(item => item.match(/\.(s?css|sass)$/)))] = outputPath
+          manifest[stripPrefix(input)] = outputPath

-> preferring index.scss from "frontend", so the external index.css does not "win", fixes the problem here.

[jaredcwhite]

Thanks @zealot128 for catching that! Would you mind applying that fix in a PR? I should be able to get merge it into the upcoming 1.2 beta.