Skip to content
This repository has been archived by the owner on Jan 25, 2020. It is now read-only.

Update munger.js #50

Closed
wants to merge 1 commit into from
Closed

Update munger.js #50

wants to merge 1 commit into from

Conversation

xjamundx
Copy link

Currently if there's a template error in kraken we sometimes get this error:

Cannot find module './'
    at Function.Module._resolveFilename (module.js:325:15)
    at Function.Module._load (module.js:276:25)
    at Function.m._load (/Users/jamuferguson/dev/paypal/p2pnodeweb/node_modules/

With this change we now get the following instead:

Could not load components/component-header-footer/header/vx-header: Cannot find module './'
    at /Users/jamuferguson/dev/paypal/p2pnodeweb/node_modules/engine-munger/lib/munger.js:77:11
    at BufferList.<anonymous> (/Users/jamuferguson/dev/paypal/p2pnodeweb/node_modules/engine-munger/view/dust.js:53:24)
    at BufferList.<anonymous> (/Users/jamuferguson/dev/paypal/p2pnodeweb/node_modules/bl/bl.js:16:14)

It's arguably way more helpful.

Currently if there's a template error in kraken we sometimes get this error:

```
Cannot find module './'
    at Function.Module._resolveFilename (module.js:325:15)
    at Function.Module._load (module.js:276:25)
    at Function.m._load (/Users/jamuferguson/dev/paypal/p2pnodeweb/node_modules/
```

With this change we now get the following instead:

```
Could not load components/component-header-footer/header/vx-header: Cannot find module './'
    at /Users/jamuferguson/dev/paypal/p2pnodeweb/node_modules/engine-munger/lib/munger.js:77:11
    at BufferList.<anonymous> (/Users/jamuferguson/dev/paypal/p2pnodeweb/node_modules/engine-munger/view/dust.js:53:24)
    at BufferList.<anonymous> (/Users/jamuferguson/dev/paypal/p2pnodeweb/node_modules/bl/bl.js:16:14)
```
@grawk
Copy link
Member

grawk commented May 11, 2016

Much better!

@grawk
Copy link
Member

grawk commented May 11, 2016

Couple unit tests failing. I can help look into it tomorrow if necessary.

@xjamundx
Copy link
Author

Thanks @grawk the actual root issue here that caused me to even add this was dust trying to find some missing content. Trying to figure out if there's an even better way to display that error message.

I'll try to fix the tests locally and rebase though.

@xjamundx
Copy link
Author

xjamundx commented May 11, 2016

I found a further root cause. Will close and continue. It's looking like this line here should probably throw instead of callback:
https://github.com/krakenjs/localizr/blob/v1.x/lib/bundle.js#L70

And then furthermore I think this in that section looks like this:

{
  file: '/Users/jamuferguson/dev/paypal/p2pnodeweb/locales',
  type: '',
  name: 'locales'
}

Which should alert us to the fact that we're not really able to read the properties file. Trying to figure out if we can fail even earlier than that.

And here's where we pass that faulty "property file path":
https://github.com/krakenjs/engine-munger/blob/v0.2.x/view/dust.js#L43

We should not be defaulting to i18n.contentPath here as this is expecting the path to a specific property file and will fairly silently fail.....Working to find the best alternative!

@xjamundx
Copy link
Author

There's a slightly better solution here:
#51

@xjamundx xjamundx closed this May 11, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants