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

Change missing property errors to warnings? #475

Closed
ccampbell opened this issue Apr 12, 2017 · 6 comments
Closed

Change missing property errors to warnings? #475

ccampbell opened this issue Apr 12, 2017 · 6 comments
Labels

Comments

@ccampbell
Copy link

After upgrading to the latest version of Svelte I was met with a bunch of Component was created without expected data property 'whatever' errors that broke my application.

It is not a huge amount of work to set default values for the properties using the data() method, but I personally liked that previously you could do things like this where your component can be hidden by default until you explicitly set({ show : true }) on it:

<div class="something{{ show ? ' show' : '' }}">Hello</div>

I find it convenient to not have to pass show="false" when initializing the component. I think for an outsider coming to svelte for the first time they would expect it to work the same way as I did (any property you reference is treated as undefined until you set a value to it) just cause that is how variable values and object properties work in javascript in general.

Anyway I think it would make sense to change this to a warning instead of breaking your whole application.

@ccampbell ccampbell changed the title Change missing property errors to warnings Change missing property errors to warnings? Apr 12, 2017
@Rich-Harris
Copy link
Member

Yep — I think it makes sense for this to be a warning

@PaulBGD
Copy link
Member

PaulBGD commented Apr 12, 2017

If you have a lot of components where you don't initially declare a data property, you'll lose out in terms of performance since the shape of the state object changes after it's initially created.

@ccampbell
Copy link
Author

ccampbell commented Apr 12, 2017

On a somewhat related note, I just noticed this:

if ( !( 'Math' in this._state ) ) { throw new Error( "Component was created without expected data property 'Math'" ); }

With a component where I am doing style="{{ Math.max(0, somethingElse) }}". I can’t reproduce in the REPL though, does that run in dev mode?


That one shouldn’t even be throwing a warning 😄

@Rich-Harris
Copy link
Member

Good point, that's a bug. It should ignore the whitelisted globals. No, the REPL doesn't run in dev mode — issue here: sveltejs/v2.svelte.dev#45

Agree on the performance point, though I don't think that's a good enough reason to force users into doing it, I think a warning is probably still appropriate.

@ccampbell
Copy link
Author

@PaulBGD in a lot of my cases I want the component to change after it is created. For example I may have a component containing an SVG where the path I’m drawing requires some additional data to be calculated asynchronously. So I have been rendering the component and then after the data is available (after making an ajax call or what have you), I am updating the state to include the extra data. I admit that is probably not a common thing.

@Rich-Harris do you want me to make a separate ticket for the Math error?

@Conduitry
Copy link
Member

Submitted a PR that makes this a warning, and also takes care of spurious the warnings about using globals like Math.

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

No branches or pull requests

4 participants