krakenjs / makara

An internationalization module for kraken and express
Other
135 stars 42 forks source link

Common resource keys #11

Closed mderrick closed 9 years ago

mderrick commented 10 years ago

I know the docs says this:

"If you have content you want to share across pages, then you should factor out use of that content into a separate partial and use that partial to achieve content sharing."

But I don't believe this makes any sense. As an example, I simply would like the word 'login' to use a common.login key. Surely it would be breaking the principles of DRY to replicate this key based on the location in which you are using it and likewise it is overkill to create a partial for one individual key.

I was wondering if this limitation is ever likely to change or if you have good reason for it that I am unaware of.

Thanks! Matt

totherik commented 10 years ago

We had planned to add a feature to support something along the lines of an import or include pragma, which could allow one to reference other bundles. Would that contribute to solving this problem/question?

mderrick commented 10 years ago

This does sound like it might, how do you envisage it being used?

Ideally I would want to just use common.* seamlessly in any template.

totherik commented 10 years ago

the thought was that any given property bundle could merely include another property bundle. Something along the lines of:

// @import "base/common"
foo=bar
baz=YOLO

If that doesn't fulfill what you're looking for, we're all still really interested in hearing about more use-cases/constraints.

lmarkus commented 10 years ago

Would it be a good idea to have an "implied" import? eg: reserved_name.properties is always included

Sounds an awful lot like globals :fearful:

mstuart commented 10 years ago

Anyone working on this currently? If not, I can help. This would be a huge win for keeping it DRY. Especially for common words like "Submit", "Next", etc. I understand the argument that you can take that common word and refactor it out to yet another dust partial w/ an associated properties file, but partials shouldn't be the only answer. It'd be great to have import statements like Erik posted above... // @import "base/common"!! /cc @bluelnkd @xjamundx

xjamundx commented 10 years ago

A weirder way would be a sense of inheritance, where you could inherit from something else and automatically get (and be able to override its properties). Probably @import is better though ;-)

jeffharrell commented 10 years ago

@mstuart No, it is not actively being worked on. If you want to pick it up that would be awesome!

I would just say to comment back on this issue with a design once you have a handle on it.

mstuart commented 10 years ago

Cool, thanks!

But, I'd like to know why we moved away from the @useContent helper in the first place? I can see how there can be performance problems, but as long as you're easy on file I/O and caching common bundles, it shouldn't be terrible.

Just wanted to make sure this PR isn't going to be denied and this is something you really want.

jeffharrell commented 10 years ago

@useContent was dynamic in that it expected the content to exist as part of the page model. Since we moved to doing content at build time and removing the need to send the content over the wire with each request, we adopted @pre instead.

When you ask about @useContent are you specifically looking for being able to access content at runtime, or being able to load arbitrary content bundles? Each is a distinct problem, but I think the latter is more pressing (and what you're looking for). The solve should still be build time IMO, so what about offering up something like {@pre type="content" bundle="common" key="foo" /} with an optional bundle attribute that allows you to override the bundle?

mstuart commented 10 years ago

I basically wanted to use it as an import statement during build-time, so it ensures that those extra bundles are available for the @pre tags to use. Lately, I haven't seen a use case for content bundles being sent down to the UI so I think this is fine (it's super wasteful anyways). Now it's just a matter of style. I like your suggestion too but wanted to offer up another alternative where you specify your imports at the top. Let me know what you think and I'll run with it

A. Import statement w/ @pre tags

(Template filename - addCard.dust)

{@import bundles="common=common, transfer=bank/transfer" /}
or
// @import "common=common, transfer=bank/transfer"

<h2>{@pre type="content" key="greeting" /}</h2>
<button>{@pre type="content" key="common.next" /}</button>
<p>{@pre type="content" key="transfer.help" /}</p>

B. Optional bundle attribute

(Template filename - addCard.dust)

<h2>{@pre type="content" key="greeting" /}</h2>
<button>{@pre type="content" bundle="common" key="next" /}</button>
<p>{@pre type="content" bundle="bank/transfer" key="help" /}</p>
elankeeran commented 10 years ago

@mstuart I tried both your options but none of them are working. Is there anything else that I need to take care. And I am using latest code.

jeffharrell commented 10 years ago

@elankeeran this issue is still open so @mstuart's sample code is a proposal for an interface. Have thoughts around it?

elankeeran commented 10 years ago

@jeffharrell I would go with the #2 option, that way I don't have to unnecessarily import the bundle in page top. Let me know the tentative date for delivery?

mstuart commented 10 years ago

@elankeeran I'm hoping to deliver it in the next couple of weeks... so mid-April.

kellyrmilligan commented 10 years ago

hello, any progress on this @mstuart, any way I can help? I definitely want to have a file at the root of US/en or something where for a specific locale you can have a set of common properties, then if I want to overwrite in a template I can. IMO I don't think a developer should have to import it them selves in the template.

mstuart commented 10 years ago

Hi @kellyrmilligan, I haven't been able to find time to work on this, yet. I can probably carve out some time at the end of the month. If you want to work on this earlier, feel free! I can help collaborate with you on it and provide my thoughts/feedback.

kellyrmilligan commented 10 years ago

absolutely. we definitely need to be able to specify a common set of properties. If you can provide some general direction/what you were thinking high level, we can both fork and collaborate that way.

pbhadauria2000 commented 10 years ago

Hi @mstuart , I modified the code and got the option B working. Got mocha test cases taken care as well. I am currently going through the grunt task ("dustjs-18n".js file). In order to get it working, I will have to change a bit more than I anticipated. Are you open for some re-designing of the grunt task to achieve the same functionality?

erikmueller commented 10 years ago

@mstuart and @pbhadauria2000, glad that this is being worked on. I'd also opt for Option B. Looking forward for the integration :+1: Any time frame foreseeable?

scobo commented 10 years ago

+1 for Option B. :+1:

aredridel commented 10 years ago

We're considering an "include" option in the bundles themselves, as an option C:

include=../../US/en/bundle.properties
pbhadauria2000 commented 10 years ago

Guys, I have a working Option B code. Will be sending pull request within a week

aredridel commented 9 years ago

We're actually swinging back toward dynamism and using @useContent, with a loader to bind messages to a source.