ractivejs / rvc

RequireJS loader plugin for Ractive components
20 stars 10 forks source link

Error with Ractive 0.10.* #19

Closed p3k closed 5 years ago

p3k commented 6 years ago

I am running rvc 0.5 fine with Ractive 0.9.10 – however, after upgrading to 0.10.0 (and greater) I get the following error when rendering a component:

Uncaught TypeError: Cannot read property '2' of undefined
    at parse (rvc.js:862)
    at make (rvc.js:929)
    at load (rvc.js:1234)
    at rvc.js:1360
    at rvc.js:1099
    at XMLHttpRequest.xhr.onreadystatechange (rvc.js:1176)

Is this due to the update of the template format in 0.10?

Template positions in parsed template AST are now stored in the q member rather than the p member to avoid accidental overlap with local partials.

fskreuz commented 6 years ago

Can I have a code sample for poking purposes? If it can't fit in a jsFiddle, a repo would be fine as well.

p3k commented 6 years ago

hi @fskreuz – sorry not being able to provide a test case in either way.

however, i gave a try patching the p property with q in the respective line 862 in dist/rvc.js and so far everything works again:

862c862
<               var contentStart = source.indexOf('>', scriptItem.p[2]) + 1;
---
>               var contentStart = source.indexOf('>', scriptItem.q[2]) + 1;

as this seems to be coming from rcu.js i checked this repo and it seems the change in ractive 0.10 is already reflected there.

rcu.js is currently at version 0.11.1, rvc.js 0.5.0 depends on version 0.10.3.

i bumped the rcu version in rvc’s package.json for a test spin, rebuilt and also with the resulting dist file the issue does not occur anymore.

28c28
<     "rcu": "^0.10.3",
---
>     "rcu": "^0.11",

would that be enough information to fix the issue?

p3k commented 6 years ago

bump

evs-chris commented 6 years ago

That sounds like it should be enough to fix the issue. If you'd like, I can add you as a maintainer so you aren't stuck waiting on us to get things fixed.

p3k commented 6 years ago

ok, that sounds interesting. however, i would like to be sure some other folks verify and review before i commit any nonsense… what’s the workflow here?

evs-chris commented 6 years ago

I'm not particularly familiar with rvc/rcu, but I know the ractive/parser side pretty well. For other sub-projects, we still do PRs with review requests any time the submitter wants a review or other feedback, and I think that would work pretty well here too.

p3k commented 6 years ago

ok, so let’s try it 😺

fskreuz commented 6 years ago

@p3k Hi! Sorry, been swamped lately. So yes, that bug is caused by rvc having embedded an older version of rcu. rcu was updated to support the AST adjustment. I have updated rvc edge branch, bumping all outdated dependencies up and rebuilt rvc. Lemme know if it's all good and I can promote it to master and publish to npm.

p3k commented 6 years ago

oh ok, just refetched the edge version and did a test run: looking good from my side, at least regarding this issue 😺

re other changes looks like some caching mechanism was implemented and it’s possible to use ractive mustaches in css now?

LeJared commented 5 years ago

@fskreuz edge version works fine. Can you please merge it and publish to npm and bower?

MartinKolarik commented 5 years ago

Published in rvc@0.6