Skip to content
This repository has been archived by the owner on Dec 5, 2022. It is now read-only.

Template Strings #79

Closed
wants to merge 1 commit into from
Closed

Template Strings #79

wants to merge 1 commit into from

Conversation

zeke
Copy link
Contributor

@zeke zeke commented Mar 22, 2016

I personally like using template strings because it's easier than concatenating strings and variables together, especially when your target string contains quotes. I'm not sure how the rest of the team feels, though, so I changed a few instances here to get a conversation started.

What do you think, @electronjs/core?

@kevinsawicki
Copy link
Contributor

What do you think

I like this 👍

I think it could be worth using more ES6 niceties as well such as const and let in the app to show people what language features are supported out of the box in Electron.

@anaisbetts
Copy link

We use template strings everywhere in the Slack Desktop app, to the point that we've rigged ESLint to warn when you concatenate strings manually

@nathansobo
Copy link

👍 to using new ES6 features wherever we can.

@jlord
Copy link
Contributor

jlord commented Mar 23, 2016

The reason there is currently no ES6 in the app is because the goal of this is to be as approachable as possible. Using extra libraries that require specialized knowledge or using not widely adopted syntax, I believe, leads us further away from approachability.

That being said, this is an application for demonstrating what Electron can do and one of those special things is using ES6 without a pre-compiler. But I want to make sure we keep ourselves in check and don't pepper in things we like because we like them but because they are useful, approachable ways that we can demonstrate what users can do with Electron.

using new ES6 features wherever we can

I am 👎 on this sentiment because I don't think it's aligned with the goals of this app in particular.

If we feel template strings and const and let meet the level of show what Electron can do without making the code hard to understand for most people then I'm 👍 with them.

@nathansobo
Copy link

Are const and let actually difficult to understand? They just seem like good practices to me.

@jlord
Copy link
Contributor

jlord commented Mar 23, 2016

Are const and let actually difficult to understand?

To me it's not about are the concepts of const and let difficult to understand but since in this app we won't be defining what they mean, will most users to this app already know what they are?

EDIT—Also, just to clarify I'm not against adding const and let in particular as I mentioned above but wanted to clarify why we ought not to use ES6 "wherever" we can in the app without a bit of consideration about what we add.

@zeke
Copy link
Contributor Author

zeke commented Mar 23, 2016

I'm happy to see both sides represented here. From an approachability standpoint, I think there's something to be said for vanilla ES5. More people are familiar with it, and won't be intimidated or confused by code snippets that contain new ES syntax.

Flip side: There are number of things in ES6 that actually make code easier to read and understand. I think const is great because it indicates that the value won't change, making it easier to grok how a program behaves. Template strings reduce cognitive load because you don't have to squint and figure out if you got the right combination of single quotes, double quotes, and plusses.

My vote would be to pepper in the ES6 niceties that we think help improve code clarity, and avoid syntax (like maybe arrow functions, rest, and spread) that might just make the code more confusing to newcomers.

@nathansobo
Copy link

I do see @jlord's point and honestly I don't really care that much, but here's an example:

// ES5
arrayA.splice.apply(arrayA, [5, 3].concat(arrayB))

// ES6
arrayA.splice(5, 3, ...arrayB)

Is the first call more approachable than the second? I'd argue it's the opposite. ES5 has major usability issues that could be quite hostile to someone who is coming to Electron with no prior JS experience. I view ES6 as a solution to a lot of these problems, though I do admit there is a subset of its features that fall more on the "bells and whistles" end of the spectrum.

All this said, maybe you just keep the examples simple enough to avoid any of these nasty corners. If you do encounter them though, I think you should go with the more readable option for a general audience, regardless of assumptions about past JS experience.

But anyway, I trust both of your judgment and see the value of balancing "better" with "more familiar".

@anaisbetts
Copy link

I think certain features are more likely to cause confusion to others - like, people are gonna see template strings and be like "Oh, of course that's what it does". On the other hand, features like destructuring or arrows are I think more likely to cause confusion. ES5 devs who see something like:

import { power } from 'electron';

or

app.on('ready', (...args) => {
});

are gonna be thrown for a loop, but like:

console.log(`the path is ${pathToFile}`);

might not be initially familiar, but people will be like, "Oh cool that's a new thing I know now"

@lee-dohm
Copy link
Contributor

So it sounds like there are multiple potential groups that have conflicting sets of things that are "unclear". Could we use this as an opportunity to educate people? Perhaps for anything that isn't typical ES5 (or anything we want/have to use that we feel might be confusing to one camp or another) we have a section in the demo for, "Hey, what's this strange syntax?" with links to relevant documentation?

@zeke
Copy link
Contributor Author

zeke commented Mar 25, 2016

Thanks for the input, everyone. Closing this in favor of #86

@zeke zeke closed this Mar 25, 2016
@zeke zeke deleted the template-strings branch March 25, 2016 22:44
haacked added a commit that referenced this pull request Sep 27, 2016
See discussion in #79
for more context.
@haacked haacked mentioned this pull request Sep 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants