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

Latest commit broke css injection #387

Open
simonwimell opened this issue Dec 1, 2018 · 17 comments
Open

Latest commit broke css injection #387

simonwimell opened this issue Dec 1, 2018 · 17 comments
Assignees
Labels

Comments

@simonwimell
Copy link

The most recent commit (the merging of pull request #385 caused the css to stop injecting with browsersync on my localhost. So, saving a scss file compiles and saves the css, but doesn't refresh the styling with browsersync. A manual page reload shows the styling though.

How to troubleshoot?

@garretthyder
Copy link
Collaborator

Hi @simonwimell

Sorry about that, sounds like the touch command had detrimental results with browsersync.

You can simply revert the touch command by dropping this line (line 132 - https://github.com/JeremyEnglert/JointsWP/blob/master/gulpfile.js#L132);
.pipe(touch())

But would you mind trying this gulpfile.js, it preserves the touch and moves the browsersync reload.
https://gist.github.com/garrett-eclipse/2fbfbe195261bbe4fa16f707d3843f31

Let me know if it works nicely and I'll create a PR.

Thanks

@mushbrain666
Copy link

Same here.

Removing touch solves the 'cant pipe to undefined' issue, however my fresh installation with your provided gulpfile still didn't work until I commented out the motion-ui stuff to clear deprecation warning errors.

Not sure what to make of it just passing on my experience.

@garretthyder
Copy link
Collaborator

however my fresh installation with your provided gulpfile still didn't work until I commented out the motion-ui stuff to clear deprecation warning errors.

Thanks for testing @mushbrain666
The motion-ui warnings are simply warnings that can be ignored and aren't related to this. The main thing I'm curious about with the gulpfile.js there is if it fixes browsersync while keeping the touch command. If so then I'll make a PR on that change but will wait on @simonwimell to also give it a try.

Appreciated

@jackfearing
Copy link

jackfearing commented Dec 7, 2018

Hello all - I just tried garret-eclipse recent gulp.js file and it does work using the .pipe(touch()); command. Trick for me was removing the browserSync stream command from the style task and placing it in the browsersync task.

@garretthyder
Copy link
Collaborator

garretthyder commented Dec 7, 2018

Thanks @jackfearing I appreciate the extra eyes and happy you noted the same reason for my re-organized gulpfile.js it was specifically to "remov[e] the browserSync stream command from the style task and plac[e] it in the browsersync task." I'll test it properly myself when i can and will PR this fix unless there's issues @simonwimell and @mushbrain666 still have with the proposed gulpfile.js (ths one i made into a gist).
Cheers

@garretthyder garretthyder self-assigned this Dec 7, 2018
@Shipwreck
Copy link

Shipwreck commented Dec 8, 2018

Fresh installation with gist gulpfile - I think I'm running into something related to #282 with browsersync reloading before changes are saved:

[13:33:38] Starting 'styles'...
[Browsersync] Reloading Browsers...
[13:33:41] Finished 'styles' after 3.35 s

Assuming the adjustments from #282 are in garrett's gist in this thread.

Can someone confirm that the ordering is mixed up and the reload should be last? Or is that how it's supposed to look?

I've noticed that sometimes it works. Maybe one time in ten. I've seen some other gulpfiles browser-sync have delays in them - is that an option here? Curious if it's possible that if everyone in #282 tried ten times they would get a certain amount of successful reloads.

I could make a screen capture for proof if the 'sometimes loading' thing seems ridiculous.

garretthyder pushed a commit to garretthyder/JointsWP that referenced this issue Dec 10, 2018
@garretthyder
Copy link
Collaborator

Hi @Shipwreck

Thank you for taking a look here, I appreciate the input. I did some tests myself and found so far the files have saved with changes prior to reload. Looking at the logic it seems because the 'touch()' command is part of the tasks to update the timestamps this triggers the onchange moments prior to the end of the task. As the touch is the last step in the process the contents of the files should be saved and compiled (at least they have so far for me).

Let me know if you find your saved changes aren't being reflected after the reload. It seems the console order is correct as the reload occurs as part of the styles task execution.

I've created a PR (#389) for further review, my branch can be found here for this issue if you want to test using it or push changes towards it;
https://github.com/garrett-eclipse/JointsWP/tree/387-touchy-browsersync

Please take a look as well @simonwimell and @mushbrain666, I'd like to avoid pushing any more breaking changes.

All the best

@jncunha
Copy link

jncunha commented Dec 22, 2018

I still can't get browsersync to work. I've tried the latest gulpfile from Garret but it's not working. Still, if I manually refresh the page the styles get updated.

I get this output:

[17:16:29] Starting 'styles'...
DEPRECATION WARNING: Passing a string to call() is deprecated and will be illegal
in Sass 4.0. Use call(get-function("shake")) instead.

DEPRECATION WARNING: Passing a string to call() is deprecated and will be illegal
in Sass 4.0. Use call(get-function("shake")) instead.

DEPRECATION WARNING: Passing a string to call() is deprecated and will be illegal
in Sass 4.0. Use call(get-function("spin")) instead.

DEPRECATION WARNING: Passing a string to call() is deprecated and will be illegal
in Sass 4.0. Use call(get-function("spin")) instead.

DEPRECATION WARNING: Passing a string to call() is deprecated and will be illegal
in Sass 4.0. Use call(get-function("wiggle")) instead.

DEPRECATION WARNING: Passing a string to call() is deprecated and will be illegal
in Sass 4.0. Use call(get-function("wiggle")) instead.

[Browsersync] Reloading Browsers...
[17:16:30] Finished 'styles' after 1.74 s

I already tried to disable and enable cache in Chrome via the Network tab but it doesn't make a difference. If I save the file twice it will work. The first will compile the styles and the second save (since there were no changes) successfully updates the styles in my browser.

@garretthyder
Copy link
Collaborator

Hi @jncunha

@JeremyEnglert merged PR #389 recently (thanks Jeremy), can you try with master;
https://github.com/JeremyEnglert/JointsWP
*@simonwimell and @mushbrain666 would love your retest too.

If there's still an issue would love if you could spawn a new ticket and I'll take a look.

Cheers

@mushbrain666
Copy link

Working much better. The terminal responses look completely orderly now.

Thanks so much @garrett-eclipse

@garretthyder
Copy link
Collaborator

Awesome thanks for reporting back @mushbrain666

@n1smo
Copy link

n1smo commented Feb 15, 2019

That shit is still broken, how hard can it be to fix this within 2 month??? I was able to fix this by simply removing the touch command, but with this latest commit its completley broken....

@garretthyder
Copy link
Collaborator

garretthyder commented Feb 22, 2019

Hi Michael (@n1smo) it sounds like you have a unique issue as others are reporting the issue resolved.

Can you open a new ticket for investigation providing your console log and any details you have. I would suggest you use more professional language and tone in that ticket or those who volunteer support for this project will probably not care to assist you.

Also note, this is an opensource project so work and support is done on a volunteer basis in contributors free time so 2 months is relatively quick in the greater scheme of things.

Cheers

@simonwimell
Copy link
Author

@garrett-eclipse Sorry for my late reply here, but I still have issues even with this new and updated gulp file. In my most recent project, using the most recent version, the styles aint processed at all. Using the previous gulp file version and just removing line 132 works:

.pipe(touch())

Do you want me to raise another issue?

@garretthyder
Copy link
Collaborator

Hi @simonwimell before we reopen or spawn a new ticket I wonder if you ran npm install again after updating the project as the touch command was introduced and package.json was updated. But if it was a pre-existing project then gulp-touch-cmd wouldn't be in place so would fail on that specific line.
Can you try running npm install and then the watch/browsersync commands and see if that was the issue. Otherwise we can probably re-open or open a new if we don't want to bother everyone on this thread. Would love the console output and any reproduction steps you have.
Thanks

@markushasselryd
Copy link

markushasselryd commented May 21, 2019

Hi @garrett-eclipse, I too have the same issue. Fresh install of JointsWP but the css is not getting injected. This is my terminal output:

[Browsersync] Watching files...
[07:54:36] Starting 'styles'...
[Browsersync] Reloading Browsers...
DEPRECATION WARNING: Passing a string to call() is deprecated and will be illegal
in Sass 4.0. Use call(get-function("shake")) instead.

DEPRECATION WARNING: Passing a string to call() is deprecated and will be illegal
in Sass 4.0. Use call(get-function("shake")) instead.

DEPRECATION WARNING: Passing a string to call() is deprecated and will be illegal
in Sass 4.0. Use call(get-function("spin")) instead.

DEPRECATION WARNING: Passing a string to call() is deprecated and will be illegal
in Sass 4.0. Use call(get-function("spin")) instead.

DEPRECATION WARNING: Passing a string to call() is deprecated and will be illegal
in Sass 4.0. Use call(get-function("wiggle")) instead.

DEPRECATION WARNING: Passing a string to call() is deprecated and will be illegal
in Sass 4.0. Use call(get-function("wiggle")) instead.

[07:54:39] Finished 'styles' after 2.81 s

I'm forced to save a second time or hit Shift+Command+R (chrome) to update the page.
Been using JointsWP for around 10 projects now and love it. Good work! But since the last update on gulpfile.js, browsersync has not been working as expected. Been using an old version to get it to work. I'm happy to assist with any troubleshooting if I can.

@garretthyder garretthyder reopened this Dec 4, 2019
@mushbrain666
Copy link

Whats the happenings with this @garretthyder? I'm about to jump in to a new project and wondering whats the current status of jointswp. I see you reopened this and I'm noticing my browsersync is not working on the last project I did with the current gulpfile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants