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

Throw causes entire script to exit if selected blocks don't exist #68

Open
mattclough1 opened this issue Aug 1, 2017 · 7 comments
Open

Comments

@mattclough1
Copy link

throw new Error("The elements you're trying to select don't exist.");

Is this intentional behavior? I can't help but feel like maybe it should just console.warn and return. In my case, the module is included in bundle, and since some pages don't have the selected elements, it causes all the JS to exit.

@moeamaya
Copy link
Member

moeamaya commented Aug 7, 2017

@mattclough1 Can you instantiate rellax only on the pages you need?

@mattclough1
Copy link
Author

@moeamaya yeah, I just went ahead and checked whether the element was available before instantiating... depending on how someone is using Rellax, an exception is probably appropriate. In my case it's embellishment. Disregard! 😅

@moeamaya
Copy link
Member

moeamaya commented Aug 7, 2017

Hey I know how it goes when you're trying to ship your site, so no worries.

I think i am going to keep the throw because I think it's best practice and breaking JS is maybe a feature in this case.

@moeamaya moeamaya closed this as completed Aug 7, 2017
@hitaspdotnet
Copy link

Similar issue without bundling

@KeironLowe
Copy link
Contributor

KeironLowe commented Mar 27, 2019

@moeamaya Respectfully, I disagree with throwing an exception. Rellax is a nice to have and typically isn't required for the page to work so throwing an exception and stopping the entire script from executing doesn't make sense to me. Rellax should definitely not run if no elements exist, but I don't believe stopping other JS from running is right.

I think throwing a warning instead would be more appropriate.

@moeamaya
Copy link
Member

moeamaya commented Mar 27, 2019

@KeironLowe How dare you say Rellax is "a nice to have" it's as critical as babel 😜

But okay I hear you all, and I recant my original position. If anyone wants to make a PR happy to merge without the blocking exception.

@KeironLowe
Copy link
Contributor

@moeamaya Some would say even more critical! Thanks, I've just submitted a PR.

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

No branches or pull requests

4 participants