romanschejbal / gassetic

Assetic replacement based on the gulp build tool
MIT License
177 stars 17 forks source link

Rewrite into JS #63

Closed romanschejbal closed 7 years ago

romanschejbal commented 7 years ago

I've rewritten the code into JS so it's more accessible for the community.

With some breaking changes, though: 1) gulp-plumber is now included automatically for proper error handling 2) gulp-livereload is not part of the code anymore, so please add it manually into your pipeline if where you need it. i.e.

tasks:
  - { name: "livereload", args: { start: true } }

3) htmlTag value is now mandatory and needs to be filled in config

arendjantetteroo commented 7 years ago

@romanschejbal Sounds great, but i've got some problems getting this to work. Can you help me?

I've did an npm install with the rewrite branch. Then did some local installs (babel-register, seems missing in package.json), but it now complains on :

can't pass { loose:true } to babel-preset-2015.

Really interested in the #62

romanschejbal commented 7 years ago

@arendjantetteroo hey thanks for volunteering! I've added the missing babel pkg.. You'll still have to install all your gulp-plugins separately tho... Let me know if you need any help!

Steps: 1) git clone gassetic 2) switch to the rewrite branch 3) run npm i or yarn inside the gassetic folder 4) run npm link to link gassetic into globals 5) run gassetic build as you normally do inside your project folder

arendjantetteroo commented 7 years ago

@romanschejbal Thanks, i'll try that out. I assumed you could just do npm install gassetic.git and then it would work. Seems that it's not the case :)

arendjantetteroo commented 7 years ago

@romanschejbal Ok that works. I get lots of these errors though when running it:

[12:00:59] Updating templates 📝 (node:9153) Warning: Possible EventEmitter memory leak detected. 11 change listeners added. Use emitter.setMaxListeners() to increase limit

And it builds in 17s while the previous version is a lot quicker, that takes around 4-5seconds.

romanschejbal commented 7 years ago

@arendjantetteroo thanks for the feedback!

A couple questions: 1) What version of node are you on please? 2) Are those 17s the time logged by gassetic or did you time it yourself? What was the longest running task? (it logs the time for each task once finished)

arendjantetteroo commented 7 years ago

Looks like addng the watchers is the slow part in dev.

arendjantetteroo commented 7 years ago

node -v v6.9.1

it's running gassetic without any commands (so the watch task).

[12:10:43] Loading gassetic.yml [12:10:44] ▸ compiling js with [12:10:44] ▸ compiling images with [12:10:44] ▸ compiling css with [12:10:44] ▸ compiling fonts with [12:10:44] ▸ compiling jstree with [12:10:44] ▸ compiling select2 with [12:10:44] ▸ compiling sass with sass [12:10:44] ✓ jstree finished in 0.14s [12:10:44] ▸ watching css/fe_basevendor.css [12:10:44] ✓ select2 finished in 0.16s [12:10:44] ▸ watching js/i18n.js [12:10:44] ✓ sass finished in 0.19s [12:10:44] ▸ watching datatables.css [12:10:44] ▸ watching fe_basehas.css [12:10:44] ✓ fonts finished in 0.2s [12:10:44] ▸ watching fonts [12:10:44] ✓ css finished in 0.21s [12:10:44] ▸ watching fe_basevendor.css [12:10:45] ▸ watching fe_dashboard.css [12:10:45] ▸ watching signup.css [12:10:46] ▸ watching fe_login.css [12:10:46] ▸ watching dashboard.css [12:10:47] ▸ watching management.css [12:10:47] ▸ watching grouptree.css [12:10:48] ▸ watching fullcalendar.css [12:10:48] ▸ watching sortable_list.css [12:10:49] ✓ images finished in 4.67s [12:10:49] ▸ watching images [12:10:49] ▸ watching img [12:10:49] ✓ js finished in 4.73s [12:10:49] ▸ watching fe_headvendor.js [12:10:49] ▸ watching fe_basevendor.js [12:10:49] ▸ watching fe_basehas.js [12:10:50] ▸ watching fe_turnover.js [12:10:50] ▸ watching fe_login.js [12:10:51] ▸ watching comment.js [12:10:51] ▸ watching hasadmin.js [12:10:52] ▸ watching hasadmin-fieldform.js [12:10:52] ▸ watching hasmanagement.js [12:10:52] ▸ watching dashboard.js [12:10:53] ▸ watching opportunity.js [12:10:53] ▸ watching mainuser.js [12:10:54] ▸ watching moveleads.js [12:10:54] ▸ watching datatables.js [12:10:54] ▸ watching leadlisttablehelper.js [12:10:55] ▸ watching chanceSteps.js [12:10:55] ▸ watching haas_admin.js [12:10:56] ▸ watching signup.js [12:10:56] ▸ watching grouptree.js [12:10:57] ▸ watching leadpolls.js [12:10:57] ▸ watching dashboardfilters.js [12:10:57] ▸ watching slots.js [12:10:58] ▸ watching todopage.js [12:10:58] ▸ watching funnelpage.js [12:10:59] ▸ watching fe_account.js [12:10:59] ▸ watching googleCalendar.js [12:10:59] ▸ watching favourites.js [12:11:00] ▸ watching start-page.js [12:11:00] Updating templates 📝 (node:10442) Warning: Possible EventEmitter memory leak detected. 11 change listeners added. Use emitter.setMaxListeners() to increase limit [SNIP lots of those errors] [12:11:01] Creating gassetic.dump.dev.yml [12:11:01] Build finished in 17.96s 👍

romanschejbal commented 7 years ago

Right, yeah it seems like the watchers take time... Just to be sure, try running gassetic build as that'll skip the watch and let's see if it's faster?

arendjantetteroo commented 7 years ago

gassetic build [13:00:56] Loading gassetic.yml [13:00:57] ▸ compiling js with [13:00:57] ▸ compiling images with [13:00:57] ▸ compiling css with [13:00:57] ▸ compiling fonts with [13:00:57] ▸ compiling jstree with [13:00:57] ▸ compiling select2 with [13:00:57] ▸ compiling sass with sass [13:00:57] ✓ jstree finished in 0.17s [13:00:57] ✓ select2 finished in 0.17s [13:00:57] ✓ sass finished in 0.19s [13:00:57] ✓ fonts finished in 0.2s [13:00:57] ✓ css finished in 0.21s [13:00:57] ✓ images finished in 0.22s [13:00:57] ✓ js finished in 0.26s [13:00:57] Updating templates 📝 [13:00:59] Creating gassetic.dump.dev.yml [13:00:59] Build finished in 2.13s 👍

arendjantetteroo commented 7 years ago

Can i easily add the manifest creation with a gulp plugin or is that planned?

romanschejbal commented 7 years ago

What manifest? You mean the one from #62? That would be a Symfony bundle that'd use the gassetic.dump.dev.yml file newly created by this version of gassetic.

romanschejbal commented 7 years ago

@arendjantetteroo thanks for your help! 👍 I'll investigate a little bit more on the watchers later on.. For now I've just bumped up the listener limit to get rid of the warning.

arendjantetteroo commented 7 years ago

Ok, that is expecting a json file, not a yml file. But that should not make a big difference as long as the yml file is parseable. I'll try it

arendjantetteroo commented 7 years ago

The maxListeners at least removes the error messages. thanks

arendjantetteroo commented 7 years ago

Another issue with the dump files. In dev i get

Prod:

So its adding a >- in prod, which is hard to handle in the yml parser (if it's even valid)

romanschejbal commented 7 years ago

@arendjantetteroo I'd think that YAML is the standard in Symfony land, and since the configuration is in YAML, I thought that the dump should be too... I've fixed the - >- line wrappers.

Thanks

arendjantetteroo commented 7 years ago

@romanschejbal when are you planning to merge this and create a new version? I want to upgrade my team to the new version so i can use the yml dumper with #62

romanschejbal commented 7 years ago

I just needed to test it for any regressions.. Since it's working fine for you, I will release the new version sometime next week. On Fri, 30 Dec 2016 at 11:35, Arend-Jan Tetteroo notifications@github.com wrote:

@romanschejbal https://github.com/romanschejbal when are you planning to merge this and create a new version? I want to upgrade my team to the new version so i can use the yml dumper with #62 https://github.com/romanschejbal/gassetic/issues/62

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/romanschejbal/gassetic/pull/63#issuecomment-269756452, or mute the thread https://github.com/notifications/unsubscribe-auth/ABU6bGLkO9uC-5en3tQv3LhWrB1vX0YSks5rNN6KgaJpZM4K60HB .

arendjantetteroo commented 7 years ago

@romanschejbal Any news?

arendjantetteroo commented 7 years ago

Woohoo, thanks!

arendjantetteroo commented 7 years ago

Looks like something went wrong with the release, see #64