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

Alternative syntax for &attributes #1744

Open
danhper opened this issue Nov 20, 2014 · 7 comments
Open

Alternative syntax for &attributes #1744

danhper opened this issue Nov 20, 2014 · 7 comments

Comments

@danhper
Copy link

danhper commented Nov 20, 2014

I tend to use the &attributes syntax quite often,
and think it is kind of verbose. The & token does not seem to be
used for anything else right now, so would not it be possible
to use an alternative syntax such as

h1.foo(data-a="b")&({'data-b': 'c'})

A small change to the lexer should be sufficient. Modifying attributesBlock
as follow works and passes all the tests.

  attributesBlock: function () {
    var captures;
    if (captures = (/^&attributes/.exec(this.input) || /^&/.exec(this.input))) {
      this.consume(captures[0].length);
      var args = this.bracketExpression();
      this.consume(args.end + 1);
      return this.tok('&attributes', args.src);
    }
  }

If you have any interest in this proposition, I'm up for testing this feature
and sending a PR.

Thanks.

@monolithed
Copy link

+1 for attr or any char like &, + etc

@ForbesLindesay
Copy link
Member

The problem is that:

  1. it makes the code harder to read (in my opinion) and I'm worried it makes the language harder for beginners to learn. Making the language easier to learn is something I think should be a really high priority for jade as many issues on here result of lack of awareness for how features work.
  2. I'm not convinced this feature should be used that much. It results in poorer performance because it relies on merging objects at runtime. It is also much harder to use with proper security than conventional attributes (you have to be careful to prevent cross-site scripting). Encouraging people to use it all over the place seems like a bad idea.

@ForbesLindesay
Copy link
Member

Lets copy jsx & ES7 object spread here and add support something like:

h1.foo(data-a="b" ...({'data-b': 'c'}))

Lets make the new ... syntax safer than &attributes by having it escape the values passed into it, and then deprecate &attributes

We can then support a special "unescaped" version that could be something like:

h1.foo(data-a="b" ...!({'data-b': 'c'}))

or alternatively (preferably) track when the values are already coming from the attributes variable passed in to mixins.

@TimothyGu
Copy link
Member

I'm reassigning this to 3.0.0 since 2.0.0 is about to be published and this is not top-priority.

@aleclarson
Copy link

aleclarson commented May 10, 2018

I think the ... syntax fits perfect!

It would be cool if mixins could use ... alone as a shorthand for ...attributes.

mixin test
  div(...)

Sooo much better than:

mixin test
  div&attributes(attributes)

@ForbesLindesay
Copy link
Member

@aleclarson I think I'd be happy with that. We could make:

mixin test
  div(...)

effectively a shorthand for:

mixin test
  div(...!attributes)

The ! indicates "unescaped" so is normally really dangerous, but we know that mixin attributes are already escaped, so it's not actually dangerous here (as long as we check it hasn't been re-assigned or anything.

@EdwinFajardoBarrera
Copy link

Hello, @ForbesLindesay I'm starting in open-source contributions and I would like to contribute to this issue. I'll appreciate it if you can provide me with architectonical guidelines!

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

No branches or pull requests

6 participants