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

Support querySelector and querySelectorAll #6

Closed
avanderhoorn opened this issue Apr 8, 2017 · 10 comments
Closed

Support querySelector and querySelectorAll #6

avanderhoorn opened this issue Apr 8, 2017 · 10 comments
Labels

Comments

@avanderhoorn
Copy link

I just tried out gulp-rcs for a static site that I'm ceating and everything seems to work well except that the js parser doesn't seem to support querySelector and querySelectorAll. My guess is that its because it doesn't match 'xxx' or "xxx". In the below cases it can have a leading . or # and that there can be more complex patterns (i.e. .xxx .ttt).

Here is my sample thats currently failing:

    var simple_dots = document.querySelector('#js-carousel');
    var dot_count = simple_dots.querySelectorAll('.js_slide').length;
    var dot_container = simple_dots.querySelector('#js-slider-indicators');
@JPeer264
Copy link
Owner

JPeer264 commented Apr 9, 2017

Thanks for your issue. Hm that is weird, it should compile. Does everything else in the javascript file compile right?

Moreover, are #js-carousel, .js_slide and #js-slider-indicators included in any precompiled css file? Because gulp-rcs, and any other rcs-core plugin, will compile all id's and classes based on the css files.

@avanderhoorn
Copy link
Author

I think I found the problem. You were correct in suggesting that those id/classes weren't a part of any style sheet, there were just present in the HTML and JS... at first glance when things weren't working, this stood out, hence why I suggested it. The real problem comes down to:

    preloadSlide('#js-carousel-slide-1', '/img/front-page/hero1.svg', function () {
        carousel.className += ' is-initialized';
    });

vs

    preloadSlide('#js-carousel-slide-1', '/img/front-page/hero1.svg', function () {
        carousel.className += 'tU';
    });

In the above you can see that rcs isn't preserving white in the strings in js files. I was thinking of just going ' ' + 'is-initialized'; to work around this bug, bug given the asset generation pipeline, uglify has already run by this stage and would have optimized the above back to ' is-initialized';.

Any chance you think this might be an easy fix?

@JPeer264
Copy link
Owner

Oh yes, that is indeed a bug. I will try to fix this asap.

Btw, usually it is recommended to run gulp-rcs or similar before uglifying the code - so I am not yet sure if there would be some other bugs, if so please let me know.

@JPeer264 JPeer264 added the bug label Apr 10, 2017
@avanderhoorn
Copy link
Author

Will do and thanks for your help! I'll see what I can do to switch tasks around.

@JPeer264
Copy link
Owner

A possible workaround for now would be following:

...
    preloadSlide('#js-carousel-slide-1', '/img/front-page/hero1.svg', function () {
        var whitespace = ' ';

        carousel.className += whitespace + 'is-initialized';
    });
...

@avanderhoorn
Copy link
Author

Ya I thought about that, have to see what it produces.

@avanderhoorn
Copy link
Author

WHOOT! Thanks for the quick response. Will this require a rev to the package gulp-rcs to get this update or just this one?

@JPeer264
Copy link
Owner

Nope, I will update gulp-rcs in couple of minutes (I'll notify you). I just had troubles with some other dependencies in Node v<6 😅

@JPeer264
Copy link
Owner

@avanderhoorn you can now install gulp-rcs@1.0.1. Happy coding ✌️ - and please let me know, if you have any other issues 👍

@avanderhoorn
Copy link
Author

Huge thanks!!! 👍

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

2 participants