can.view Breaking backward compatibility in v2.0.5 amd #717

Closed
steeleprice opened this Issue Feb 5, 2014 · 21 comments

Comments

Projects
None yet
3 participants
@steeleprice

I am having a major issue with v2.0.5.

I have a working App that has had no other change except a straight replacement of v2.0.4 with v2.0.5 AMD.

These statements used to work:

$(element).html(can.view(require.toUrl("comp/home/home.html"), {}));
$('body').append(can.view.mustache("<app-login headers='session.securityHeader' token='session.token' notifications='session.notifications' rememberme='session.persistToken'></app-login>")({ session: attr.session }));

Now they both fail in scanner.js at line 249 with Object doesn't support property or method 'replace'

This is because source is a document fragment.
I can get around the first by first saving the view to a variable then setting html to the frag, but this seems weird to have to do when I never had to previously.
The second sample is even used in all the documentation for Components.
Am I missing something that documents this and need to change my syntax?
I'd consider this a pretty big deal since I would have to change at least dozens of places views are loaded.

Thanks

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 5, 2014

Contributor

Does it work if your template ends with .mustache ?

Sent from my iPhone

On Feb 4, 2014, at 11:24 PM, Steele Price notifications@github.com wrote:

I am having a major issue with v2.0.5.

I have a working App that has had no other change except a straight replacement of v2.0.4 with v2.0.5 AMD.

These statements used to work:
$(element).html(can.view(require.toUrl("comp/home/home.html"), {}));
$('body').append(can.view.mustache("")({ session: attr.session }));

Now they both fail in scanner.js at line 249 with Object doesn't support property or method 'replace'

This is because source is a document fragment.
I can get around the first by first saving the view to a variable then setting html to the frag, but this seems weird to have to do when I never had to previously.
The second sample is even used in all the documentation for Components.
Am I missing something that documents this and need to change my syntax?
I'd consider this a pretty big deal since I would have to change at least dozens of places views are loaded.

Thanks


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Feb 5, 2014

Does it work if your template ends with .mustache ?

Sent from my iPhone

On Feb 4, 2014, at 11:24 PM, Steele Price notifications@github.com wrote:

I am having a major issue with v2.0.5.

I have a working App that has had no other change except a straight replacement of v2.0.4 with v2.0.5 AMD.

These statements used to work:
$(element).html(can.view(require.toUrl("comp/home/home.html"), {}));
$('body').append(can.view.mustache("")({ session: attr.session }));

Now they both fail in scanner.js at line 249 with Object doesn't support property or method 'replace'

This is because source is a document fragment.
I can get around the first by first saving the view to a variable then setting html to the frag, but this seems weird to have to do when I never had to previously.
The second sample is even used in all the documentation for Components.
Am I missing something that documents this and need to change my syntax?
I'd consider this a pretty big deal since I would have to change at least dozens of places views are loaded.

Thanks


Reply to this email directly or view it on GitHub.

@steeleprice

This comment has been minimized.

Show comment
Hide comment
@steeleprice

steeleprice Feb 5, 2014

Let me try that, but it seems to only be an issue in AMD.
I created a fiddle using latest/can and it works as expected.
I really need to create a fiddle template soon for AMD, but I can't do it today.

Let me try that, but it seems to only be an issue in AMD.
I created a fiddle using latest/can and it works as expected.
I really need to create a fiddle template soon for AMD, but I can't do it today.

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Feb 5, 2014

Contributor

What happens if you just use

$(element).html(can.view("comp/home/home.html", {}));

This could have been introduced because of #647

Contributor

daffl commented Feb 5, 2014

What happens if you just use

$(element).html(can.view("comp/home/home.html", {}));

This could have been introduced because of #647

@steeleprice

This comment has been minimized.

Show comment
Hide comment
@steeleprice

steeleprice Feb 5, 2014

.mustache didn't work, it's still a fragment instead of a string as it is in 2.0.4.

daffl: that works, but is really strange, require.toUrl just changes the path to "scripts/app/components/home/home.html"
if I pass anything but a string literal, it's failing, i.e.

                var template = require.toUrl("comp/home/home.mustache");
                $(element).html(can.view(template, {}));

which shows template is a string variable containing: "scripts/app/components/home/home.mustache"
It seems related to #647 but isn't a require base path problem, the problem is that anything but view("string literal", {}) is not rendering.

This fails too:

                var template = "scripts/app/components/home/home.html"
                $(element).html(can.view(template, {}));

.mustache didn't work, it's still a fragment instead of a string as it is in 2.0.4.

daffl: that works, but is really strange, require.toUrl just changes the path to "scripts/app/components/home/home.html"
if I pass anything but a string literal, it's failing, i.e.

                var template = require.toUrl("comp/home/home.mustache");
                $(element).html(can.view(template, {}));

which shows template is a string variable containing: "scripts/app/components/home/home.mustache"
It seems related to #647 but isn't a require base path problem, the problem is that anything but view("string literal", {}) is not rendering.

This fails too:

                var template = "scripts/app/components/home/home.html"
                $(element).html(can.view(template, {}));
@steeleprice

This comment has been minimized.

Show comment
Hide comment
@steeleprice

steeleprice Feb 5, 2014

in line 652 of view.js, using the above syntax,
text is undefined and id is a fragment.

in line 652 of view.js, using the above syntax,
text is undefined and id is a fragment.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 5, 2014

Contributor

I built an example that works.

<html>
    <body>
        <div id='element'></div>
        <script src="require.js"></script>
        <script>
            require(['can','jquery'], function(can, $){
                $('#element').html(can.view("home.html", {}));


                $('body').append(can.view.mustache("Hello")({ session: {} }));
            });
        </script>
    </body>
</html>
Contributor

justinbmeyer commented Feb 5, 2014

I built an example that works.

<html>
    <body>
        <div id='element'></div>
        <script src="require.js"></script>
        <script>
            require(['can','jquery'], function(can, $){
                $('#element').html(can.view("home.html", {}));


                $('body').append(can.view.mustache("Hello")({ session: {} }));
            });
        </script>
    </body>
</html>
@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 5, 2014

Contributor

Changing it to:

var str = "home.html"
$('#element').html(can.view(str, {}));

works too

Contributor

justinbmeyer commented Feb 5, 2014

Changing it to:

var str = "home.html"
$('#element').html(can.view(str, {}));

works too

@steeleprice

This comment has been minimized.

Show comment
Hide comment
@steeleprice

steeleprice Feb 5, 2014

wait a minute...
I'm inside a Component, but I think I see what is going on.
There is an async require for google maps in a sub component that is timing out.

wait a minute...
I'm inside a Component, but I think I see what is going on.
There is an async require for google maps in a sub component that is timing out.

@steeleprice

This comment has been minimized.

Show comment
Hide comment
@steeleprice

steeleprice Feb 5, 2014

no, even removing that isn't helping, but I do think this is Component related, most of the Components are loading when stepping through.

no, even removing that isn't helping, but I do think this is Component related, most of the Components are loading when stepping through.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 5, 2014

Contributor

@steeleprice ping me on skype / ghangout if you get your code to a minimal breaking case. I was just about to update the site to show 2.0.5. But I want to hold off if it's breaking.

Contributor

justinbmeyer commented Feb 5, 2014

@steeleprice ping me on skype / ghangout if you get your code to a minimal breaking case. I was just about to update the site to show 2.0.5. But I want to hold off if it's breaking.

@steeleprice

This comment has been minimized.

Show comment
Hide comment
@steeleprice

steeleprice Feb 5, 2014

I'm working on a fiddle to emulate it.
I have something close, but it's working, I will adjust it in a few minutes.

I'm working on a fiddle to emulate it.
I have something close, but it's working, I will adjust it in a few minutes.

@steeleprice

This comment has been minimized.

Show comment
Hide comment
@steeleprice

steeleprice Feb 5, 2014

ok, so it appears that views don't work quite the same as they used to, but only inside of a component. I am OK with this since all it required was directly calling mustache.
this worked in 2.0.4 inside of a component:

$(element).html(can.view(require.toUrl("comp/home/home.html"), {}));

it doesn't in 2.0.5.
but this does and is better syntax anyway:

$(element).html(can.view.mustache(require.toUrl("comp/home/home.html"))({}));

I still have this error being thrown with the following line though and I am trying to track down exactly what is causing it, this line did in fact render correctly in 2.0.4.

$("body").append(can.view.mustache("<app-location></app-location>")({ session: attrs.session }));

This seems to be a tightening of what is accepted for rendering which is mostly a good thing.
debugging the root cause is quite painful.

ok, so it appears that views don't work quite the same as they used to, but only inside of a component. I am OK with this since all it required was directly calling mustache.
this worked in 2.0.4 inside of a component:

$(element).html(can.view(require.toUrl("comp/home/home.html"), {}));

it doesn't in 2.0.5.
but this does and is better syntax anyway:

$(element).html(can.view.mustache(require.toUrl("comp/home/home.html"))({}));

I still have this error being thrown with the following line though and I am trying to track down exactly what is causing it, this line did in fact render correctly in 2.0.4.

$("body").append(can.view.mustache("<app-location></app-location>")({ session: attrs.session }));

This seems to be a tightening of what is accepted for rendering which is mostly a good thing.
debugging the root cause is quite painful.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 5, 2014

Contributor

@steeleprice I'm not sure you are correct. The following works for me:

<html>
    <body>
        <div id='element'></div>
        <script src="require.js"></script>
        <script>
            require(['can','jquery'], function(can, $){

                var str = "home.html"
                $('#element').html(can.view(require.toUrl("home.html"), {}));


                $('body').append(can.view.mustache("Hello")({ session: {} }));
            });
        </script>
    </body>
</html>

Please reach out to me so I can stop worrying about this and make 2.0.5 official.

Contributor

justinbmeyer commented Feb 5, 2014

@steeleprice I'm not sure you are correct. The following works for me:

<html>
    <body>
        <div id='element'></div>
        <script src="require.js"></script>
        <script>
            require(['can','jquery'], function(can, $){

                var str = "home.html"
                $('#element').html(can.view(require.toUrl("home.html"), {}));


                $('body').append(can.view.mustache("Hello")({ session: {} }));
            });
        </script>
    </body>
</html>

Please reach out to me so I can stop worrying about this and make 2.0.5 official.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 5, 2014

Contributor

Wait ... why would you do:

$(element).html(can.view(require.toUrl("comp/home/home.html"), {}));

inside a component? This seems like a mistake. You should not be rendering things inside a component.

Contributor

justinbmeyer commented Feb 5, 2014

Wait ... why would you do:

$(element).html(can.view(require.toUrl("comp/home/home.html"), {}));

inside a component? This seems like a mistake. You should not be rendering things inside a component.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 5, 2014

Contributor

Btw, I am writing an article about how we are going to be backwards compatible / use semantic versioning: https://gist.github.com/justinbmeyer/8812069

So if there is a breaking issue, I want to fix it ASAP so I'm not undermining the entire article :-)

Contributor

justinbmeyer commented Feb 5, 2014

Btw, I am writing an article about how we are going to be backwards compatible / use semantic versioning: https://gist.github.com/justinbmeyer/8812069

So if there is a breaking issue, I want to fix it ASAP so I'm not undermining the entire article :-)

@steeleprice

This comment has been minimized.

Show comment
Hide comment
@steeleprice

steeleprice Feb 5, 2014

this is definitely a combination problem of Require and setting Component templates.
the rendering inside the component is not the issue, but I will address that too.
The problem was how template: was being called.
ALL my components used:
template: can.view(require.toUrl("comp/home/home.html")),
which in 2.0.5 is
A) no longer needed since it can use a require path directly
and
B) a breaking change, because if you used it in a previous version it's now trying to resolve the path twice.
correct syntax for 2.0.5 is:
template: can.view("comp/home/home.html"),

as for rendering in the component, normally, yes, this is not done, <app-components> are inside the template. In fact, right now <app> doesn't have a template and I am most likely going to change that shortly. I will have to play with that approach. This is all a conversion from a non-component architecture where everything was controls before.
If you have a single root <app> component, it may need to so some rendering for initial setup that is potentially outside the <app> tag

I have to put login, logout and location finders in the body so they don't get clobbered when the hash changes and replaces content inside the <app> tag.

There is really no perfect way to do something like this, it's going to be a trade-off somewhere.
if I created an index page something like this:

<app />
<app-login />
<app-logout />
<app-location />

then I have to reference app.scope.session from inside the other 3 components which now tightly couples them to an app component.
if they are instantiated inside of <app>, they need to be placed in the DOM outside of the tag since routing will replace things inside the tag.

If I want <app> as a root component (as opposed to a control), I am not seeing a good way, except maybe using

<div id="nav">
<app-nav-bar/>
</div>
<div id="content" />
<div id="otherstuff">

inside the template.

This is where Documentation of sample architectures for real-world apps is going to be enormously beneficial.

We are guessing at best-practices right now using "Todos" as a template when we need to think of things outside that, like security, session observes, communication pipelines between components without a facade or sandbox (pub/sub), etc.

this is definitely a combination problem of Require and setting Component templates.
the rendering inside the component is not the issue, but I will address that too.
The problem was how template: was being called.
ALL my components used:
template: can.view(require.toUrl("comp/home/home.html")),
which in 2.0.5 is
A) no longer needed since it can use a require path directly
and
B) a breaking change, because if you used it in a previous version it's now trying to resolve the path twice.
correct syntax for 2.0.5 is:
template: can.view("comp/home/home.html"),

as for rendering in the component, normally, yes, this is not done, <app-components> are inside the template. In fact, right now <app> doesn't have a template and I am most likely going to change that shortly. I will have to play with that approach. This is all a conversion from a non-component architecture where everything was controls before.
If you have a single root <app> component, it may need to so some rendering for initial setup that is potentially outside the <app> tag

I have to put login, logout and location finders in the body so they don't get clobbered when the hash changes and replaces content inside the <app> tag.

There is really no perfect way to do something like this, it's going to be a trade-off somewhere.
if I created an index page something like this:

<app />
<app-login />
<app-logout />
<app-location />

then I have to reference app.scope.session from inside the other 3 components which now tightly couples them to an app component.
if they are instantiated inside of <app>, they need to be placed in the DOM outside of the tag since routing will replace things inside the tag.

If I want <app> as a root component (as opposed to a control), I am not seeing a good way, except maybe using

<div id="nav">
<app-nav-bar/>
</div>
<div id="content" />
<div id="otherstuff">

inside the template.

This is where Documentation of sample architectures for real-world apps is going to be enormously beneficial.

We are guessing at best-practices right now using "Todos" as a template when we need to think of things outside that, like security, session observes, communication pipelines between components without a facade or sandbox (pub/sub), etc.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 5, 2014

Contributor

Ok, I tracked this down to this pull request:

#647

As supporting baseUrl was never documented, this is a new feature and should have been part of 2.1 or 3.0.

@stevenvachon and @daffl I think this is still not documented, anyway we can add this to can.view's docs?

Contributor

justinbmeyer commented Feb 5, 2014

Ok, I tracked this down to this pull request:

#647

As supporting baseUrl was never documented, this is a new feature and should have been part of 2.1 or 3.0.

@stevenvachon and @daffl I think this is still not documented, anyway we can add this to can.view's docs?

@justinbmeyer justinbmeyer added this to the 2.1.0 milestone Feb 5, 2014

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Feb 5, 2014

Contributor

@steeleprice

Don't you mean correct syntax for 2.0.5 is:

template: can.view(require.toUrl("comp/home/home.html")),

Sorry about the break. We are trying very hard to prevent that.

Contributor

justinbmeyer commented Feb 5, 2014

@steeleprice

Don't you mean correct syntax for 2.0.5 is:

template: can.view(require.toUrl("comp/home/home.html")),

Sorry about the break. We are trying very hard to prevent that.

@steeleprice

This comment has been minimized.

Show comment
Hide comment
@steeleprice

steeleprice Feb 5, 2014

No, this is a GOOD change and just needs to have some documentation to close this issue.
correct syntax is:
template: can.view("comp/home/home.html"),
comp is a requireJS path, not just the base.
in my require setup it look like this:

require.config({
    baseUrl: 'scripts',
    paths: {
        app: 'app',
        controllers: 'app/controllers',
        comp: 'app/components',
        can: 'lib/can',
...

No, this is a GOOD change and just needs to have some documentation to close this issue.
correct syntax is:
template: can.view("comp/home/home.html"),
comp is a requireJS path, not just the base.
in my require setup it look like this:

require.config({
    baseUrl: 'scripts',
    paths: {
        app: 'app',
        controllers: 'app/controllers',
        comp: 'app/components',
        can: 'lib/can',
...

@daffl daffl modified the milestones: 2.0.6, 2.1.0 Feb 10, 2014

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Feb 25, 2014

Contributor

So #718 adds documentation for this. If anybody thinks it is inadequate please reopen this issue.

Contributor

daffl commented Feb 25, 2014

So #718 adds documentation for this. If anybody thinks it is inadequate please reopen this issue.

@daffl daffl closed this Feb 25, 2014

@steeleprice

This comment has been minimized.

Show comment
Hide comment
@steeleprice

steeleprice Feb 26, 2014

There is only one other odd thing I noticed in this issue, but its not really the same issue and would also be solved with documentation.

self closing tags have unexpected behavior in most browsers.
they nest as children of the last element instead if siblings as they should.
Its also inconsistent and I will need to open a new issue for it.

There is only one other odd thing I noticed in this issue, but its not really the same issue and would also be solved with documentation.

self closing tags have unexpected behavior in most browsers.
they nest as children of the last element instead if siblings as they should.
Its also inconsistent and I will need to open a new issue for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment