Sage: [Sage 9] Node v6.x path issue

[dani-z]

Submit a feature request or bug report

• [ ] This is a feature request
• [x] This is a bug report
• [x] This request isn't a duplicate of an existing issue
• [x] I've read the docs and NPM Debugging Guidelines post and followed them (if applicable)
• [ ] This is not a personal support request that should be posted on the Roots Discourse forums

---

What is the current behavior?

npm run build:production is failing

What is the expected or desired behavior?

Build should work.

---

Bug report

(delete this section if not applicable)

Please provide steps to reproduce, including full log output:

Using Node v6.x path is expecting a string TypeError: Path must be a string. Received { js: 'scripts/main_ba96f3fab55a5e191f46.js' }
It is used here:

var assetsPluginProcessOutput = function (assets) {
var name,
ext,
filename,
results = {};

for (name in assets) {
if (assets.hasOwnProperty(name)) {
if (path.extname(assets[name]) === '') {
for (ext in assets[name]) {
if (assets[name].hasOwnProperty(ext)) {
filename = name + '.' + ext;
results[filename] = path.basename(assets[name][ext]);
}
}
}
}
}
return JSON.stringify(results);
}

Please describe your local environment:

WordPress version: latest

OS: latest

NPM/Node version: v6.0.0 / 3.9.0

Where did the bug happen? Development or remote servers?

Dev server, when running npm run build:production

Is there a related Discourse thread or were any utilized (please link them)?

Haven't found one

---

Other relevant information:

Replacing path.extname() with path.format() (which is expecting an object) will fix the issue.

[AdamWills]

I've had problems with using newer versions of node. Using nvm to get working with node 0.12.x usually takes care of these issues (you may have to clear out your node_modules folder and run npm install again with 0.12.x).

[jaredpalmer]

had same issue, except I fixed it by commenting out the line:

var assetsPluginProcessOutput = function (assets) {
  var name,
      ext,
      filename,
      results = {};

  for (name in assets) {
    if (assets.hasOwnProperty(name)) {
      // if (path.extname(assets[name]) === '') {
        for (ext in assets[name]) {
          if (assets[name].hasOwnProperty(ext)) {
            filename = name + '.' + ext;
            results[filename] = path.basename(assets[name][ext]);
          }
        }
      // }
    }
  }
  return JSON.stringify(results);
}

[regenrek]

As @jaredpalmer pointed it out, this is the line where it fails.

Created a forum thread about that. Didn't expected that this is a bug.
https://discourse.roots.io/t/whats-the-purpose-of-assetspluginprocessoutput/6851

Regards

[ptrckvzn]

That was on my list of issues with the current webpack implementation.

This function assetsPluginProcessOutput in webpack.config.js is there to filter the default output of webpack AssetsPlugin and output a format consistent with what Sage JsonManifest.php and Asset.php (src/libSage/) expect.

I think the best solution would be to update src/libSage/Asset functions to use the default AssetsPlugin output. Which looks like that :

{
    "main":{
        "js":"scripts/main_c6444901f85d626269ea.js",
        "css":"styles/main_c6444901f85d626269ea.css"
    },
    "customizer":{
        "js":"scripts/customizer_c6444901f85d626269ea.js"
    }
}

This way, the assetsPluginProcessOutput function could be removed completely.

Maybe some feedback from @QWp6t who worked on Asset.php and JsonManifest.php would help decide the next step.

[retlehs]

I think the best solution would be to update src/lib/Sage/Asset functions to use the default AssetsPlugin output.

agreed — that's a decent chunk of the webpack config removed :+1:

we can update the json manifest to account for this

[aaemnnosttv]

Not sure if this should be a separate issue or not, but seems possibly related. I noticed that build:production was failing when trying to use Font Awesome imported in main.scss.

// Import npm dependencies
@import "~bootstrap/scss/bootstrap";
@import "~font-awesome/scss/font-awesome";

The normal build script works fine. I'd be happy to open a new issue if this isn't related.

[retlehs]

@aaemnnosttv not sure either but can you try #1673 if it's not already in your codebase? i'm able to import and build font-awesome for prod, but run into issues with another npm dependency that has assets in a stylesheet

[ptrckvzn]

Initially I added this if statement because in some situation (that I can't reproduce :-/ ) the assets array get passed to the function with filename "main.js" instead of entry names ("main", "customizer"). If npm run build:productionfails, then you could try to output assets to the console and see what the function received. That would help you determine if this is the problem.

console.log(assets);
if (path.extname(assets[name]) === '') { 

[aaemnnosttv]

@retlehs build:production still failing after bringing in the change from #1673.

@ptrckvzn, I popped in the log there in assetsPluginProcessOutput and it logged this for the assets:

{ main: { js: 'scripts/main_072447d8f76f1d1725af.js' },
  customizer: { js: 'scripts/customizer_072447d8f76f1d1725af.js' } }

Error starts with:

ERROR in ./styles/main.scss
Module build failed: ModuleNotFoundError: Module not found: Error: Cannot resolve 'file' or 'directory' ../fonts/fontawesome-webfont.eot in /Users/...

Thanks guys. Really enjoying webpack so far!

Let me know if there is any more info I can provide to help.

[ptrckvzn]

Your problem is not related to this issue. The assets output is as expected. I think it would be interesting that you start a new topic on how to import Font Awesome with npm and webpack on the discourse forum. I found this in one of my projects. Does it helps ?

assets/styles/main.scss

$fa-font-path: "~font-awesome/fonts";
@import "~font-awesome/scss/font-awesome";

[aaemnnosttv]

@ptrckvzn That was it. I didn't realize I had to set that too. Thanks so much!

[QWp6t]

@ptrckvzn or anyone else, can you help me understand this issue? Can you possibly provide me with a reduced testcase? I'm on Windows, and I've tried Node v5 and v6 both 32- and 64-bit variants. I'm not seeing any of these same problems as you guys at all. Once I can get access to an environment that has this issue, I'll gladly look into getting it fixed.

This is an example of the assets.json that I get when running npm run build:production.

{
  "main.js":"main_079d6de85fa826c1f1e9.js",
  "main.css":"main_079d6de85fa826c1f1e9.css",
  "customizer.js":"customizer_079d6de85fa826c1f1e9.js"
}

Update: Just installed node v6 on my vagrant box and retested. Same thing. No issues. So I'm going to need more information on this one.

[ptrckvzn]

Actually, I am not able to reproduce the issue here either, but I think it would be better to remove the assetsPluginProcessOutput function from webpack.config.js and use default assetsPlugin json output in Sage Json Manifest. I added this function at first only because I did not want to touch the php files while working on the webpack config and I wanted webpack to output the same json format as Sage 8.

[QWp6t]

Okay, I'll see about refactoring it while I work on this today. I'm closing this issue since nobody can seem to reproduce it. Thanks a bunch for your response!

[mtx-z]

Sage 9.0.1 and FontAwesome 5 (@fortawesome/fontawesome-free-webfonts and see this).

package.json dependencies (yarn install)

    "@fortawesome/fontawesome": "^1.1.8",
    "@fortawesome/fontawesome-free-brands": "^5.0.13",
    "@fortawesome/fontawesome-free-regular": "^5.0.13",
    "@fortawesome/fontawesome-free-solid": "^5.0.13",
    "@fortawesome/fontawesome-free-webfonts": "^1.0.9",

Import from main.scss

@import "~@fortawesome/fontawesome-free-webfonts/scss/fontawesome";
@import "~@fortawesome/fontawesome-free-webfonts/scss/fa-solid";
@import "~@fortawesome/fontawesome-free-webfonts/scss/fa-regular";

Add variable to _variables.scss
$fa-font-path: "~@fortawesome/fontawesome-free-webfonts/webfonts";

Works with yarn run build:production.