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 applyStyles so it will work with sites with CSP that reject 'unsafe-inline' #377

Closed
wants to merge 1 commit into from

Conversation

bynormous
Copy link

Change lets sites with strict Content Security Policies so they can use FilePond without allowing unsafe:inline

@rikschennink
Copy link
Collaborator

applyStyles is called multiple times each frame, have you compared the performance of this solution to the previous version? Preferably in dev tools performance tab with 4x or 6x CPU slowdown.

@bynormous
Copy link
Author

It's about 5x slower, the older method is definitely faster - but in absolute terms, seems still fast. In my test I dragged and dropped a file and filepond created ~400 style changes. Testing the same 400 style changes with the new code only added 7ms on my machine. It scales linearly with the number of style changes and cpu slowdown. Not a huge difference but also maybe not ideal if you're not interested in supporting sites with content security policies.

btw, thanks very much for making filepond, you make the best looking libraries

@rikschennink
Copy link
Collaborator

It would be nice to support sites with strict content security policies but that's quite a lot to be losing per frame.

I wonder if storing the ['transform-origin', 'transform', 'opacity', 'visibility', 'pointer-events', 'width', 'height'] array outside of the function would make a big difference (currently you're creating it on each change).

I'm not sure why you're using forEach in the first loop and for let in the second, both can be done with forEach.

The nested splitting of strings could also be the cause of slowdown, so maybe replacing the styles string with an array will be faster?

@bynormous
Copy link
Author

Almost all the slowdown is from the multiple element.style changes. Storing the array instead of creation seems to be negligible. The array splits accounts for ~10% of the slowdown. I think performance can be improved by only resetting the props that aren't being used.

But interestingly just replacing element.setAttribute('style', styles) with element.style.cssText = styles actually doesn't trigger csp errors in Chrome. I think the properties being used are white-listed, but not sure if that's true for all browsers.

So I'll do more research and reach out again with a better solution.

@bynormous bynormous closed this Sep 12, 2019
@rikschennink
Copy link
Collaborator

Thanks @bynormous, very much appreciated!

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

Successfully merging this pull request may close these issues.

None yet

2 participants