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

string interpolation in attribute does not work as expected #58

Closed
s3ththompson opened this issue Dec 31, 2018 · 10 comments · Fixed by #93
Closed

string interpolation in attribute does not work as expected #58

s3ththompson opened this issue Dec 31, 2018 · 10 comments · Fixed by #93
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@s3ththompson
Copy link

html`<div class="${'1'} ${'2'} ${'3'}"></div>`

currently returns

VNode {
  nodeName: 'div',
  children: [],
  attributes: { class: '1' },
  key: undefined }

expected

VNode {
  nodeName: 'div',
  children: [],
  attributes: { class: '1 2 3' },
  key: undefined }
@jacksteamdev
Copy link

See #49 (comment) for a solution.

@s3ththompson
Copy link
Author

Any reason this wouldn't (or couldn't) be supported? Coming from nanohtml, this was a confusing change of behavior (especially without any warning or error).

It also seems weird that

html`<div class="x y z"></div>`

yields three classes but the interpolated version doesn't.

@developit
Copy link
Owner

developit commented Jan 7, 2019

This is something we lost in 2.0 as part of aligning the syntax with JSX

I'm open to supporting it, since we do other things JSX doesn't support (eg: unquoted attributes). I just wouldn't want it to balloon the size if we choose to support it.

FWIW for classes it might be worth using something like clsx to ensure the resulting value is normalized (when values are falsey, etc): <div class=${cx(1, 2, 3)} />

@developit developit added enhancement New feature or request help wanted Extra attention is needed labels Jan 7, 2019
@kristoferjoseph
Copy link

kristoferjoseph commented Mar 22, 2019

I really liked the previous behavior.
I inline variable expansion all over the place to keep it local to the attribute.
i.e.

    <${SidebarLink}
      active="${active === 'functions'}"
      alt="functions"
      class="${linkClasses}"
      href="/apps/${id}/functions"
    >
      Functions
    <//>
    <${SidebarLink}
      active="${active === 'environments'}"
      alt="environments"
      class="${linkClasses}"
      href="/apps/${id}/environments"
    >
      Environments
    <//>
    <${SidebarLink}
      active="${active === 'data'}"
      alt="data"
      class="${linkClasses}"
      href="/apps/${id}/data"
    >
      Data
    <//>

the current behavior lops off everything after the ${id} so href="/apps/${id}/data" gets rendered as href="/apps/123/" which is not expected.

The work around is to define these strings with another string template literal before usage, but then you end up having to reference variables defined at the top of the function instead of in the attribute you are reading, or use the ${'string' + var + 'string'} which string template literals were designed to eliminate 😂

@developit
Copy link
Owner

I guess we could base this off of the effect on size? That's the only metric we have to compare against. If it were possible to add back support for interpolated attribute values in only a few bytes, that would seem justifiable.

@cj
Copy link

cj commented Apr 18, 2019

Having support for this added back in would be awesome!

I'm sure @developit can work his magic and make it happen in less than one byte! 🎩 ✨ 💖 😄

@developit
Copy link
Owner

lol I'm more counting on @jviide's magic, he's better at this than I am

@jviide
Copy link
Collaborator

jviide commented Jun 17, 2019

+26 bytes was the best I could do. See #93.

@jviide jviide closed this as completed in #93 Jul 8, 2019
@jviide
Copy link
Collaborator

jviide commented Jul 8, 2019

The just merged PR #93 implements this functionality, adding +23 bytes to the compressed bundle size. Hopefully it works for your use case!

@jviide
Copy link
Collaborator

jviide commented Jul 29, 2019

...and this has now been released as a part of HTM 2.2 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants