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

How does data-ref get its value? #89

Closed
cruzanmo opened this issue Jul 16, 2015 · 10 comments
Closed

How does data-ref get its value? #89

cruzanmo opened this issue Jul 16, 2015 · 10 comments

Comments

@cruzanmo
Copy link

Given two resources x and y as defined below:
x:
{
  a: {
    _ref: y
  }
}

y:
{
  a: 1
}

And given this nunjucks template for y:
<article data-ref="{{ _ref }}">{{ a }}</article>

Then:

.../instances/x.html returns:
<article data-ref="y">1</article>

.../instances/y.html returns:
<article data-ref="">1</article>

Note that when you request y directly, the data-ref is empty.

Perhaps _ref should be considered the ref of itself within the context of a nunjucks template. If this is confusing with the usage of _ref elsewhere, we could use _self within the nunjucks template. This would mean data-ref would have the same value in both .../instances/x.html and .../instances/y.html.

For example, the template could be

<article data-ref="{{ _self }}">{{ a }}</article>

An advantage to this approach is that we can rely on the html to always have the data-ref defined when replacing a single component within a page, as we are currently doing in byline-editor.

A disadvantage is that there would need to be logic in byline that assumes that all users of byline would always want to have _ref or _self available in the component's html template. Currently we are only using the _ref in the templates so that byline-editor knows where components are on the page.

@cruzanmo cruzanmo changed the title How does data-ref get its value? Use _self for data-ref in templates? Jul 16, 2015
@cruzanmo cruzanmo changed the title Use _self for data-ref in templates? How does data-ref get its value? Jul 16, 2015
@nelsonpecora
Copy link
Contributor

Ah, @TakenPilot this is the same issue I had when I wanted to do stuff to the layout instance.

@TakenPilot
Copy link
Contributor

For whenever this is brought up again, these are the reasons why there will never be a _ref at the base of any object returned from byline-core:

  1. The meaning of the word reference is "something that refers" and I am very hesitant to give the word special meaning for our particular project. That is, it shouldn't suddenly change from representing "that guy over there" to "who I am" because of its position in an object tree.

Reference:
a : allusion, mention
b : something (as a sign or indication) that refers a reader or consulter to another source of information (as a book or passage)
c : consultation of sources of information

  1. In a RESTful API, GET <--> PUT is supposed to be commutative in that putting something somewhere should mean that getting the exact same uri should give the exact thing you put there, and vice versa. There are multiple places you could get the same information from, and assigning a ref into the data means you can't save to multiple places without modifying the data each time (i.e., versioning, publishing, etc.).

  2. Having a _ref means that you are pointing somewhere else, and that anything in that ref object means that it is someone else's data. Having a _ref at the base is implying that the entire object is someone else's data. Unless you're trying to create an exception?

  3. Having a _ref at the base of an object means that you can easily PUT a _ref to a uri that doesn't match; we often PUT to multiple places because of publishing and versioning, and having the uri in the data means we have to edit the data each time we were to do that. Performing a simple diff between objects becomes a multiple step process. Having an object at a certain uri with a _ref at another uri sounds like a debugging nightmare.

  4. "data-ref" only exists because at the time I was using it for that first demo to do replacement, which is the exact thing you're trying to do now. If the only reason why this is being debated is for the possibility of having non-root elements having a data-ref, then it is being used in a way that wasn't intended.

  5. "data-ref" only exists if that component's html is pointing to someone else's html. Currently this is consistent with how _ref is being used. It always refers to some other uri, not the one used to fetch the current resource (the current page).

  6. The only thing that actually cares about "data-ref" or even knows of its existence is the editor; if the editor were not on the page, we'd be better off never having data-ref in the page at all.

  7. One of the main reasons we broke up this thing into pieces instead of having another massive "factory" is to avoid stuff like this, where pieces are tightly bound together. This is also known as Law of Demeter; also known as the benefits of loose coupling; also known as single responsibility principle. The byline-core project just doesn't need to know about data-ref at all; it just composes. The multiplex template project also doesn't need to know about data-ref; it just renders. No one cares about data-ref except for the editor.

  8. Having a ref at the base of an object means that all of the code that does recursion on our objects will have to be rewritten. This includes how we do composition, how we create new pages, and how we publish data. It would have to be rewriten to ignore the first level of data and then start the recursion, which considering that most of our objects are only a single level or two levels deep, would do terrible things to our performance. For scale, that's about 60% of byline-core's codebase.

@cruzanmo
Copy link
Author

Could there be an easier solution by targeting the .html route, since that is the only place where we need the _self ref value?

I am not sure about "No one cares about data-ref except for the editor." Data-ref is how we are indicating that an element is a component, which can be used as a querySelector.

@TakenPilot
Copy link
Contributor

By whom? The editor? By client-side JavaScript? I don't think overloading the meaning of data-ref is healthy either. It's data that is a reference. If someone is assuming that a data-ref means something is a component, they're the one making an assumption. : )

Someone else could use class names. And someone else could use data-component. That last one would be the most accurate, don't you think?

I really don't think telling another project like byline-core and multiplex-template about your data-ref that's only used in yet another project is anything close to maintainable or recommended. Please help keep our projects in their own bins.

@TakenPilot
Copy link
Contributor

Example: Someone needs to implement a feature with hardcoded strings in their project. They use gulp. Instead of adding the feature in their own project, they decide to modify gulp because it was easier, putting their hardcoding strings for their own project right into gulp's code. And now they're expecting gulp to merge their PR.

You're trying to put "data-ref", your hardcoded string, into projects that have never seen that before in their life.

Note: multiplex-templates also has no idea what a _ref is. Interesting, eh?

@cruzanmo
Copy link
Author

ref is already used throughout byline. I am not suggesting pulling data-ref into byline.

What I am suggesting is to expose the ref, which byline already knows, to the render engine.

Locally, I think I have this working by adding one line of code: adding data._ref = ref; after this line https://github.com/nymag/byline/blob/master/lib/composer.js#L139

@TakenPilot
Copy link
Contributor

Right, but why? You just want to do that to make a particular other project react in the way you want, when you could do the exact same thing in this project instead. So why would byline-core want to do that? What will the docs say to justify it?

"We decided it would be cool to be special in this one case, because reasons, so deal with it." :D

I mean, you certainly can't say that you're doing it because some project that depends on this one needs this exception. That's admitting that this editor is the only main consumer of the byline-core api. That's also admitting that having that project separate really doesn't make any sense, because we've coupled it directly with the editor. If byline can only exist with this particular editor, then we should merge the projects. If that's the decision, I'm okay with that. We can merge the editor code into byline-core or vice versa, and that would actually make it easier to manage the addition and removal of the editor from templates. This might be a really good idea! Merging the editor and byline-core together would mean that I am 100% okay creating tight coupling between the code bases.

If you're going to add an exception to another project because it's easier than adding it to your current project, you're going down a weird path, right? Especially since you're only doing it because you're trying to handle a use-case that doesn't make any sense to begin with -- having a data-ref on a non-root element when their entire reason for existence is to help with the replacement of html components.

@TakenPilot
Copy link
Contributor

Let's phrase this another way:

I believe that this project and byline-core should be merged together as much as I believe that the factory and ambrose should be merged together (which, again, I'm not totally opposed to). What you're suggesting is something like this is added to the article endpoint on ambrose because it would be so very convenient for the factory:

if (data.articles.length === 1) {
  data.date = articles[0].date; 
} 

I mean, yeah, it could be that all the templates in the factory would benefit from a line like that, because we use the article endpoint so much. The guys running the ambrose project would probably scream at you though. "Why?" they would ask, "It's inconsistent!" and you might reply, "But it's so easy! Look, here's a PR."

@cruzanmo
Copy link
Author

In effect the ref is already getting passed to multiplex-templates as follows:

In lib/composer.js:

...
componentName = components.getName(ref);
...
data.template = data.template || componentName;
...

Another approach I am thinking about is using a filter to add the ref through a nunjucks filter; there might be a way to make it work with the current version of byline. @yoshokatana, is it possible to extend the embed filter in an instance of byline?

@TakenPilot
Copy link
Contributor

Ah, I see, that's... 😅 My words aren't so convincing, so... I guess submit a PR?

Though I would consider the ROI of your use-case vs how many days you're spending on this. When you get a chance, maybe run the actual problem that you're trying to solve (replacing components in the html) past @yoshokatana . Seems like its a problem that's already been solved since the first demo months ago, and this is a black hole of logic.

I'm going to close this issue, because you and @yoshokatana really need to talk, and this is a black hole of logic for a feature where you're discussing editing another project to serve this one. We should either move the discussion there as a feature request, in which case we can actually answer the request, or we should create a new issue here that discusses what actually can be done in this particular project (the editor).

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

No branches or pull requests

3 participants