avanzu / AdminThemeBundle

Admin Theme based on the AdminLTE Template for easy integration into symfony
MIT License
281 stars 149 forks source link

SpinJs - Unable to find file "@AvanzuAdminThemeBundle/Resources/public/vendor/spinjs/spin.js" #226

Closed xmontero closed 6 years ago

xmontero commented 6 years ago

There is a problem with the SpinJs path.

To reproduce with a minimal example, go to any temporal directory and create an empty symfony project version 3.3.10 and go ahead:

xavi@bromo:~/tmp$ symfony new test_avanzu 3.3.10
xavi@bromo:~/tmp$ cd test_avanzu/
xavi@bromo:~/tmp/test_avanzu$ composer require avanzu/admin-theme-bundle
xavi@bromo:~/tmp/test_avanzu$ nano app/AppKernel.php

Add new Avanzu\AdminThemeBundle\AvanzuAdminThemeBundle(), to the file, save and go ahead:

xavi@bromo:~/tmp/test_avanzu$ php bin/console avanzu:admin:fetch-vendor
xavi@bromo:~/tmp/test_avanzu$ php bin/console avanzu:admin:build-assets

boom!!

[InvalidArgumentException]
Unable to find file "@AvanzuAdminThemeBundle/Resources/public/vendor/spinjs/spin.js".

In fact, if we inspect this directory there is not any spin.js file, but a TypeScript one:

xavi@bromo:~/tmp/test_avanzu$ cd vendor/avanzu/admin-theme-bundle/Resources/public/vendor/spinjs/
xavi@bromo:~/tmp/test_avanzu/vendor/avanzu/admin-theme-bundle/Resources/public/vendor/spinjs$ ls -l
total 48
-rw-rw-r-- 1 xavi xavi  637 nov 26 21:15 CONTRIBUTING.md
-rw-rw-r-- 1 xavi xavi  472 nov 26 21:15 Gruntfile.js
-rw-rw-r-- 1 xavi xavi 1119 nov 26 21:15 LICENSE.md
-rw-rw-r-- 1 xavi xavi  771 nov 26 21:15 package.json
-rw-rw-r-- 1 xavi xavi  675 nov 26 21:15 README.md
-rw-rw-r-- 1 xavi xavi  155 nov 26 21:15 rollup.config.js
drwxrwxr-x 4 xavi xavi 4096 nov 26 21:15 site
-rw-rw-r-- 1 xavi xavi 1832 nov 26 21:15 SpinnerOptions.d.ts
-rw-rw-r-- 1 xavi xavi 8485 nov 26 21:15 spin.ts
-rw-rw-r-- 1 xavi xavi  152 nov 26 21:15 tsconfig.json

How should we proceed?

sylvaincombes commented 6 years ago

Hello !

I think this can be related about relaxing upper bound version in the bower.json (*). In spinjs, the js file is present on 2.3.2 version of the plugin but not after, see : https://github.com/fgnass/spin.js/tree/2.3.2

We can see that this as been corrected here on the master branch : https://github.com/avanzu/AdminThemeBundle/blob/master/Resources/bower/bower.json#L25 but the patch is not present on 1.3 version (or other branches) : https://github.com/avanzu/AdminThemeBundle/blob/1.3.9/Resources/bower/bower.json#L25 I think this needs an hotfix like the one on font awesome of yesterday

xmontero commented 6 years ago

Ok! I'm going to fork and do a pull-request, then.

sylvaincombes commented 6 years ago

@xmontero the pull request already exist and has been merged in master, see #199 from november 2017.

But this bugfix needs to be ported to other branches (I think but not sure :) ), like 1.3.x (hotfix-1.3.5 ?) maybe develop etc and then do a proper 1.3.10 release, maybe @shakaran can do it ?

xmontero commented 6 years ago

In fact what I see is that change 1.3.7 to 1.3.8 was precisely the spinjs correction. Unfortunately the patch for FontAwesome was "mounted" over 1.3.7 instead of 1.3.8 thus the tag 1.3.9 was loosing the change introduced in 1.3.8.

That is the only change from 1.3.7 to 1.3.8.

I'm going to perform a merge from 1.3.8 over 1.3.9 to carry on the SpinJs change and do a PR of the merge.

xmontero commented 6 years ago

Issued a Pull-Request that simply merges already existing open ends. No new code introduced. https://github.com/avanzu/AdminThemeBundle/pull/227

shakaran commented 6 years ago

@xmontero thanks for point this up. Probably I mess the merge with the last release by make the things quickly. I deleted the branch hotfix-1.3.5 and created the 1.3.x which should be used to maintain the old 1.3 versions. Master branch will be used for the new 2.x releases and "develop" or "feature/2.0" are the old branches created by the other author keeped as history if needed to check some change in future, but it will be only used as read, no new merges on it

sylvaincombes commented 6 years ago

Thank you @shakaran @xmontero