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

Checking for SVG.supported flag #4

Open
aforty opened this issue May 12, 2017 · 3 comments
Open

Checking for SVG.supported flag #4

aforty opened this issue May 12, 2017 · 3 comments

Comments

@aforty
Copy link

aforty commented May 12, 2017

Hi,
I'm having an issue in my testing environment. I include svg.js which early on does a test for whether svg's are supported in the current environment and exits out if they are not.

https://github.com/svgdotjs/svg.js/blob/04e21b18c30d546ed6a16b279504a2b58b503541/src/svg.js#L20

My issue is that svg.js will exit if not supported but svg.easing.js will not. svg.easing.js attempts to set SVG.easing[key] = easing[key] which fails because SVG.easing does not exist.

Would you accept a PR for a similar check for SVG.supported and exit the script the same way as svg.js does if it's not supported?

@Fuzzyma
Copy link
Member

Fuzzyma commented Jun 26, 2017

Sorry for the late reply!
I wonder if this really would be neccesary. After all the code just throws an error but nothing breaks.
It is ofc more clean when checking that before...hm...

@aforty
Copy link
Author

aforty commented Jun 26, 2017

I think it is necessary. If one is to use the proper method of importing depdencies via import then they cannot be wrapped in an if environment check (not to mention a try-catch). Therefore the same file cannot be run in a test environment that doesn't support SVG. Since svg.js includes this check then I think this extension script should too in order to be consistent.

@Fuzzyma
Copy link
Member

Fuzzyma commented Jun 30, 2017

The thing is, that this check would be needed in every svg.js plugin which is quite a bit of an overhead.
Adding it here would mean we need to add it everywhere.
I agree with you that it would be more consistent.

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

2 participants