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

Component arguments that are not valid JS identifiers #887

Closed
vp2177 opened this issue Oct 6, 2017 · 5 comments · Fixed by #1555
Closed

Component arguments that are not valid JS identifiers #887

vp2177 opened this issue Oct 6, 2017 · 5 comments · Fixed by #1555
Labels

Comments

@vp2177
Copy link
Contributor

vp2177 commented Oct 6, 2017

REPL
When b-c='2' is changed to, say, b_c='2' it works.

The problem is the generated code has data: {b-c: 2}, where b-c is not quoted.

@vp2177 vp2177 changed the title Arguments that are not valid JS identifiers Component arguments that are not valid JS identifiers Oct 6, 2017
@Conduitry Conduitry added the bug label Oct 12, 2017
@Conduitry
Copy link
Member

Thanks! Yeah, something should be done about this. I'm wondering though whether it might be better to instead disallow attributes like b-c on nested components. There are definitely some other places in Svelte where property names that aren't valid as .whatever keys aren't supported.

@Conduitry
Copy link
Member

I think once #1106 gets settled, we should do the same thing here.

@Conduitry Conduitry mentioned this issue Jan 15, 2018
33 tasks
@Conduitry
Copy link
Member

Conduitry commented Jan 18, 2018

I think there probably are more sweeping changes we want to make here. Data properties in components whose names aren't valid identifiers can't be accessed in template tags. It's true that get and set can still be manually used with them, but it seems worthwhile to have dev warnings if data() returns non-identifier-safe keys, or if set is called or the component constructor is called with invalid keys.

If we make those be dev warnings (and not dev exceptions), does it still make sense to disallow non-identifier-safe component arguments at compile time, or should that too be a warning and we just quote the key in the compiled component? Not sure what's best. How much limiting of the component author is helpful and how much is overbearing?

@vp2177
Copy link
Contributor Author

vp2177 commented Jan 18, 2018

Additionally, a way to access these data fields from template tags could be added, though I can't think of a nice syntax.. {{this.get("b-c")}} seems overly verbose

@Rich-Harris
Copy link
Member

My inclination here would be to disallow invalid identifiers as component properties, because there are so many caveats around their use (e.g. you can't use them in computed props, etc).

The one thing holding me back is the possibility of us introducing spread props, where leftover props could be added to an element:

<!-- App.html -->
<Widget data-foo='bar'/>

<!-- Widget.html -->
<div ...someSubsetOfProps>...</div>

fjorgemota added a commit to fjorgemota/svelte that referenced this issue Jun 22, 2018
This fixes sveltejs#887 by quoting name of attributes if those are invalid JS identifiers when passing data to nested components.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants