added can parameter #980

Merged
merged 4 commits into from May 16, 2014

Conversation

Projects
None yet
3 participants
@gsmeets
Contributor

gsmeets commented May 9, 2014

can.view.parser is set at the end of the file. The dependency can is
not imported, this causes issues during AMD optimization, because the
reference can at that point doesn't exist through window.can, as it
does in the browser.

gsmeets
added can parameter
`can.view.parser` is set at the end of the file. The dependency `can` is
not imported, this causes issues during AMD optimization, because the
reference can at that point doesn't exist through `window.can`, as it
does in the browser.
@gsmeets

This comment has been minimized.

Show comment
Hide comment
@gsmeets

gsmeets May 9, 2014

Contributor

Hmm, seems like I'm running into some more of these issues. I'll add those to this pull request once I've fixed them..

Contributor

gsmeets commented May 9, 2014

Hmm, seems like I'm running into some more of these issues. I'll add those to this pull request once I've fixed them..

@ccummings ccummings added the Bug label May 9, 2014

@ccummings ccummings added this to the 2.1.1 milestone May 9, 2014

gsmeets
added can parameter as import to stache
Several properties are set on `can` in the file. The dependency `can` is
not imported, this causes issues during AMD optimization, because the
reference `can` at that point doesn't exist through `window.can`, as it
does in the browser.
@gsmeets

This comment has been minimized.

Show comment
Hide comment
@gsmeets

gsmeets May 12, 2014

Contributor

I've added another can import for view/stache/stache.js. Now I got stache precompiled templates running on node. I've imported can/util/library.js for lack of a better candidate. Doesn't seem to matter that much, as long as there's some prototype to extend that will be picked up later by other components.

Contributor

gsmeets commented May 12, 2014

I've added another can import for view/stache/stache.js. Now I got stache precompiled templates running on node. I've imported can/util/library.js for lack of a better candidate. Doesn't seem to matter that much, as long as there's some prototype to extend that will be picked up later by other components.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer May 12, 2014

Contributor

It should not be can. That forces a dependency on everything in core. It should be 'can/util'.

Sent from my iPhone

On May 12, 2014, at 5:10 PM, Guido Smeets notifications@github.com wrote:

I've added another can import for view/stache/stache.js. Now I got stache precompiled templates running on node. I've imported can/util/library.js for lack of a better candidate. Doesn't seem to matter that much, as long as there's some prototype to extend that will be picked up later by other components.


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented May 12, 2014

It should not be can. That forces a dependency on everything in core. It should be 'can/util'.

Sent from my iPhone

On May 12, 2014, at 5:10 PM, Guido Smeets notifications@github.com wrote:

I've added another can import for view/stache/stache.js. Now I got stache precompiled templates running on node. I've imported can/util/library.js for lack of a better candidate. Doesn't seem to matter that much, as long as there's some prototype to extend that will be picked up later by other components.


Reply to this email directly or view it on GitHub.

@gsmeets

This comment has been minimized.

Show comment
Hide comment
@gsmeets

gsmeets May 13, 2014

Contributor

To clarify, for can/view/parser I just added a parameter name for the already imported can/view. For can/view/stache/stache I added can/util/library as a reference. Both imports are named can locally.

(p.s. Travis reports failed builds. Is that due to my changes? Seems unlikely to me.)

Contributor

gsmeets commented May 13, 2014

To clarify, for can/view/parser I just added a parameter name for the already imported can/view. For can/view/stache/stache I added can/util/library as a reference. Both imports are named can locally.

(p.s. Travis reports failed builds. Is that due to my changes? Seems unlikely to me.)

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer May 13, 2014

Contributor

can/util/library should be can/util.

Contributor

justinbmeyer commented May 13, 2014

can/util/library should be can/util.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer May 13, 2014

Contributor

the can/util/library might have broken it.

Contributor

justinbmeyer commented May 13, 2014

the can/util/library might have broken it.

gsmeets
changed reference from `can/util/library` to `can/util`.
changed reference from `can/util/library` to `can/util` to fix the
dependency. This does seem to work for both the steal and the amd
versions.

ccummings added a commit that referenced this pull request May 16, 2014

@ccummings ccummings merged commit 8d0df68 into canjs:master May 16, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment