Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Peaceful coexistence of user-supplied helpers #32

Closed
rragan opened this issue Oct 18, 2012 · 5 comments
Closed

Peaceful coexistence of user-supplied helpers #32

rragan opened this issue Oct 18, 2012 · 5 comments

Comments

@rragan
Copy link
Contributor

rragan commented Oct 18, 2012

Currently, the dust-helper.js implementation will blow away any existing dust.helpers the user may have created. Could the code be a bit more tolerant and add the standard ones to an existing dust.helpers if it already exists.

I guess one could assert that the dependency order needs to be gotten correct and then this is not an issue but it adds another trap people can fall into.
@rragan rragan mentioned this issue Oct 19, 2012
Closed
@vybs
Copy link
Contributor

vybs commented Oct 19, 2012

Sure, agree to make this tolerant. PR welcome:)

@seriousManual
Copy link
Contributor

as we have the same issue, i second that. :)

@jimmyhchan
Copy link
Contributor

Seems we can just change line #490 to merge into dust.helpers if it exists.
Should be a quick change. Let me play around.

@jimmyhchan jimmyhchan mentioned this issue Nov 11, 2012
Closed
@rragan rragan mentioned this issue Aug 2, 2013
Closed
mouyang added a commit to mouyang/dustjs-helpers that referenced this issue Jan 16, 2014
@mouyang
LinkedInAtticGH-32 - populate dust.helpers using iteration
278111e 
in lib/dust-helpers.js

force a failing script loading order by reworking existing specRunners
- server - sandwich dust.helpers.tapper assignment between dust and
  dust.helpers assignment
- client - use LABjs to force a specific script-loading order to achieve
  equivalent effect as the server
@mouyang
Copy link
Contributor

mouyang commented Jan 16, 2014

I worked on this yesterday and have a working test case. There are two outstanding issues that I need help with before issuing a pull request. If anyone can help, that would be appreciated.

  1. Travis CI succeeds for Node.js v0.8 but not for v0.6. This also happened with a trivial semicolon commit I made as well. Furthermore, issue Initial step to switch to grunt build.  #62 suggests this v0.6 support is going to be dropped anyways.
  2. I'm new to Node.js and thus I haven't figured out how to integrate the test case into the test runner. I browsed Node documentation and it suggests that I can only specify one test file (not two) in package.json. Figure out how to run multiple test scripts through package.json mouyang/dustjs-helpers#2 fixed.

@mouyang mouyang mentioned this issue Jan 16, 2014
Closed
mouyang added a commit to mouyang/dustjs-helpers that referenced this issue Jan 27, 2014
@mouyang
LinkedInAtticGH-32 - populate dust.helpers using iteration
37d3b84 
in lib/dust-helpers.js

force a failing script loading order by reworking existing specRunners
- server - sandwich dust.helpers.tapper assignment between dust and
  dust.helpers assignment
- client - use LABjs to force a specific script-loading order to achieve
  equivalent effect as the server
mouyang added a commit to mouyang/dustjs-helpers that referenced this issue Jan 27, 2014
@mouyang
Merge branch 'LinkedInAtticGH-32' of github.com:mouyang/dustjs-helper…
a54e140 
…s into LinkedInAtticGH-32
mouyang added a commit to mouyang/dustjs-helpers that referenced this issue Feb 7, 2014
@mouyang
squash previous commits
d17d574 
1. LinkedInAtticGH-32 - populate dust.helpers using iteration
   37d3b84

in lib/dust-helpers.js

force a failing script loading order by reworking existing specRunners
- server - sandwich dust.helpers.tapper assignment between dust and
  dust.helpers assignment
- client - use LABjs to force a specific script-loading order to achieve
  equivalent effect as the server

2. mv specRunner2.js specRunner.js
   28233cf

3. remove client spec runner
   6b1b3ba

it's now autogenerated from grunt build + enforcing a specific script load
order seems like overkill

4. import testUtils
   bde96fe
patrick-steele-idem added a commit to patrick-steele-idem/dustjs-helpers that referenced this issue Jun 19, 2014
@patrick-steele-idem
LinkedInAttic#32 LinkedInAttic#72 Merge helpers instead of reassignin…
dbd7ca2 
…g dust.helpers
@sethkinast
Copy link
Contributor

Helpers no longer clobber dust.helpers as of 1.6.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@vybs @mouyang @jimmyhchan @seriousManual @sethkinast @rragan