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.

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

Oneway mappings, and technical debt #1568

Merged
merged 5 commits into from
Dec 11, 2014
Merged

Oneway mappings, and technical debt #1568

merged 5 commits into from
Dec 11, 2014

Conversation

Rich-Harris
Copy link
Member

This follows on from the discussion on #1542. I've come to feel quite strongly that reverse mappings shouldn't be supported - they add complexity, and exist to support a niche use case that can be met using existing mechanisms.

I won't merge it until people (esp @martypdx and @evs-chris) have had a chance to weigh in, obviously.

This is reflective of a broader issue that I think we need to face. We've always prioritised 'just works' and fixing people's issues above vague notions of architectural purity or code quality. I think that's a good thing, but like all good things you can have too much of it, and I think the result is that we've accumulated quite a bit of technical debt. A concrete example is the difficulty I faced with #1566 - I still wouldn't say that I've found the root of the problem.

I'm more guilty of this than anyone - the challenge of supporting every conceivable use case is a lot more fun than tidying things up. But I believe it's time to try and face down this technical debt more aggressively than we have in the past - this PR is a starting point. If it means removing non-essential functionality, then I'm okay with that, as long as it's done judiciously.

One of the things people often say about Ractive is that they'd like to contribute more, but that the codebase is too intimidating. We can fix that.

@martypdx
Copy link
Contributor

👍 Sorry I haven't had a chance to get to this, being a juror on two-week trial has prevented anything more than brief comments on issues.

export default function Viewmodel$init () {
var key, computation, computations = [];

for ( key in this.ractive.computed ) {
computation = this.compute( key, this.ractive.computed[ key ] );
computations.push( computation );
reverseMapping( this, key );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do think we still need to validate that the computation and throw if it is? I guess computations currently can shadow defined data, but what would be the purpose of allowing this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - yes, that should be an error

@Rich-Harris
Copy link
Member Author

two-week trial

oof, you drew the short straw there!

@martypdx
Copy link
Contributor

oof, you drew the short straw there!

Yes, but I suppose it's a large deposit in my account at the bank of social karma... 😄

@evs-chris
Copy link
Contributor

+1 to paying down technical debt. Is it time to start a roadmap and/or design document or issue? I could use some guidance on what to shoot for in a slightly more cohesive format than the list of issues.

Yes, but I suppose it's a large deposit in my account at the bank of social karma

Ah, civic duty. I'll trade you a crashed ancient production server, the subsequent rebuild after the failover shuffle, and a head cold?

Rich-Harris added a commit that referenced this pull request Dec 11, 2014
Oneway mappings, and technical debt
@Rich-Harris Rich-Harris merged commit f7d5062 into dev Dec 11, 2014
@Rich-Harris Rich-Harris deleted the oneway-mappings branch December 11, 2014 15:30
@Rich-Harris
Copy link
Member Author

I tried to start a roadmap just now but realised I'm terrible at this stuff. Triaging issues into milestones will help, I think. Broadly, the kinds of things I'm thinking about:

  • 0.7.x - paying down technical debt, ensuring that mappings work as advertised and result in a performance boost
  • 0.8.x - would really like to get progressive enhancement (Progressive enhancement #395) working in some form by then
  • 0.9.x - figure out how Ractive components relate to web components

On an ongoing basis, I think we just need to do whatever we can within reason to keep SLOC count down and readability up.

Not sure how helpful this is... would be open to doing this in a more structured way, just not entirely sure how!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants