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

jQueryUI incompatible with js/app.js #28984

Closed
mikestratton opened this issue Jun 28, 2019 · 17 comments

Comments

Projects
None yet
5 participants
@mikestratton
Copy link
Contributor

commented Jun 28, 2019

  • Laravel Version: 5.8.*
  • PHP Version: 7.3.3
  • Database Driver & Version: mysqlnd 5.0.12-dev
    -- XAMPP Control Panel v3.2.3
    -- Windows 10
    -- Jetbrains PhpStorm

Description:

jQueryUI Datepicker works fine when ran outside of Laravel application. Here is the code: https://jqueryui.com/datepicker/ Implementing the same code from within any laravel blade view returns a "datepicker" not defined in the developers console(browser).

Commenting out the line:

{{--    <script src="{{ asset('js/app.js') }}" defer></script>--}}

in the app.blade.php file and the jquery date picker works.

Steps To Reproduce:

  1. Install Laravel 5.8 using the development environment stated in this message.
  2. php artisan make:auth
  3. Create a blade view with an input form for date.
  4. Add the code from https://jqueryui.com/datepicker/ to the blade view and/or app.blade.php.
  5. Run your blade view and check console - jQueryUI values are not being passed.
  6. Comment out the js/app.js as previously stated in this message.
  7. jQueryUI datepicker calendar will now work.
@570studio

This comment has been minimized.

Copy link

commented Jun 29, 2019

I just tried this out on a (relatively) fresh install of Laravel and the datepicker worked as expected without commenting out the app.js reference. But I am not using a Windows machine or XAMPP (Mac/Valet/PHP 7.2.17 instead).

That being said, I am not sure how those environment differences would cause the issue. Did you try it with a fresh install of Laravel?

@mikestratton

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2019

Yes

@mikestratton

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2019

Can also be resolved if defer is removed from the script.

<script src="{{ asset('js/app.js') }}" defer></script>
<script src="{{ asset('js/app.js') }}" ></script>
@mikestratton

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2019

@570studio Did you run php artisan make:auth ?

mikestratton added a commit to mikestratton/framework that referenced this issue Jun 30, 2019

Fixes Issue laravel#28984
Defer in javascript prevents jqueryui components from loading.

taylorotwell added a commit that referenced this issue Jun 30, 2019

Merge pull request #29001 from mikestratton/5.8
[5.8] Fixes Issue #28984, defer in app.js script prevents jquery components from loading.
@570studio

This comment has been minimized.

Copy link

commented Jun 30, 2019

@mikestratton I did run php artisan make:auth. Looks like your PR got merged though, so 👍

@dongido001

This comment has been minimized.

Copy link

commented Jul 8, 2019

Hi, @570studio

After running php artisan make:auth, did it work for you?

Now the removal of the defer breaks My Vue app. I now have to move the script to the bottom of the page.

@devcircus

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2019

Yeah the removal of defer doesn’t make sense without also moving the script tag to the bottom. Not sure why that was merged.

driesvints added a commit that referenced this issue Jul 8, 2019

driesvints added a commit that referenced this issue Jul 8, 2019

Merge pull request #29109 from laravel/revert-29001-5.8
Revert "[5.8] Fixes Issue #28984, defer in app.js script prevents jquery components from loading."
@driesvints

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

I've reverted the pr for now.

@devcircus

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2019

The state of this after the revert, is more than sufficient for a starter scaffold. Of course there will be situations where you'll need to change it, but that should be handled in the individual app as the need arises. As it is, it suffices the 80% use-case.

@mikestratton

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2019

I strongly disagree. I am using jQuery in some but not all html forms. I create a view just for this form. There is no need to compile all the required code in JavaScript when it is not widely used.

I have found a lot of mid level developers struggling with this. Just Google “jquery not working with Laravel”.

The ability to add an additional script that does not break the site should be the default working state. developers should not be required to compile everything. It should be an option and is a good coding practice to compile, but lack thereof should not break the app.

The “defer” seems to be a symptom of a much bigger problem.

Ease of use is a must and what has drawn many to laravel. Ignoring this bug will only offer continued frustration for thousands of developers.

@mikestratton mikestratton reopened this Jul 8, 2019

@mikestratton

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2019

If @taylorotwell agreed with the initial commit, why revert his commit?

@devcircus

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2019

This pr breaks the most fundamental concept of mixing javascript and html.

@mikestratton

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2019

Yes but so does the current state with defer.

@mikestratton

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2019

If moving the script to the bottom of the page allows the use of jQuery libraries, why not just do that?

@mikestratton

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2019

So, in essence, it is ok to deliver an html page that reads only one script?

@driesvints

This comment has been minimized.

Copy link
Member

commented Jul 9, 2019

We won't be reverting this. Feel free to change this in your own codebase.

@driesvints driesvints closed this Jul 9, 2019

@mikestratton

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

Ok, thanks for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.