search

froala / wysiwyg-rails

Ruby gem for Froala jQuery WYSIWYG HTML Rich Text Editor.
https://froala.com/wysiwyg-editor
MIT License
464 stars 102 forks source link

No reason to precompile all the assets #56

Closed jeffblake closed 5 years ago

jeffblake commented 6 years ago

Because the files are added via the application.js/css manifest, there's no reason to have them precompiled separately into the asset pipeline.

brendon commented 6 years ago

Please look into this. It helps with debugging if the original assets aren't already compressed.

brendon commented 6 years ago

Upon considering this, I think they're precompiled on purpose to provide obfuscation of the code given you need a special license to see the full source code. It's a shame to double-compile but perhaps Froala could provide a source-visible version of the gem to subscribers at that level using something like: https://gemfury.com ?

javierjulio commented 5 years ago

The gem includes the unminified versions of the CSS files although the JS files are only in minified form.

This issue should be reopened as this is an unnecessary and unwanted side effect of #12, which should be reverted. There is no need to create a standalone file for each and every CSS and JS file in the gem. The docs show to include any asset files in an app's existing manifest file (e.g. application.scss, application.js) which is correct.

By including wysiwyg-rails, we have more than doubled the number of files that are precompiled, despite not using any of the assets directly! We include specific assets from this gem in an existing CSS or JS manifest file. As an example, when running assets precompilation on our app, we had 591 files generated with wysiwyg-rails as is. By undoing the wysiwyg-rails precompile config, that came down to 225 files!

brendon commented 5 years ago

@javierjulio, the javascript files are definitely only included in the repo as minified. Full CSS files are provided though.

javierjulio commented 5 years ago

That's right thanks @brendon. I'll update my comment.

This issue should be reopened. I can create a new one as well. Either way these assets shouldn't be precompiled.

brendon commented 5 years ago

@stefanneculai will need to do this.