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

fix (#1484 fixing 't.fn.datepicker is undefined' when loading bootstr… #2478

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jdnierth
Copy link

Q A
Bug fix? yes
New feature? no
BC breaks? no
Related tickets mentioned #2362, #1484, #1196
License MIT

Description

  • Fixing bug 't.fn.datepicker is undefined' firing after minification.
  • Fixing bug by making check for jquery as a dependency more defensive within AMD check.
  • Setup within require.js with jquery exported (see project configuration further down)
  • jQuery version 2.2.1
  • Bootstrap Datepicker version: 1.8.0

Console.log screenshot

console-error

Project specific setup

HTML structure

 <div class="js-datepicker-container">
      <input type="text" class="form-control" id="dob" placeholder="Date Naisssance*(JJ/MM/AAAA)">
 </div>

RequireJS config excerpt

var require = {
    paths: {
        'jquery': 'Vendor/jquery-2.2.4',
        'bs-dropdown': 'Plugins/bootstrap/dropdown',
        'bs-datepicker': 'Plugins/bootstrap-datepicker/bootstrap-datepicker',
        'bs-datepicker-lang-fr': 'Plugins/bootstrap-datepicker/locales/bootstrap-datepicker.fr',
	...
    },
    shim: {
        'jquery': {exports: '$'},
        'bs-datepicker-lang-fr': [
            'jquery',
            'bs-datepicker',
        ]
    }
};

Custom module usage

define([
       	'jquery',
        'bs-datepicker-lang-fr'
    ],
    function search(
        $,
        bsDatepicker
    ) {
     
            var $datepickerContainer = $('.js-datepicker-container input');

            $datepickerContainer.datepicker({
                forceParse: false,
                format: 'dd/mm/yyyy'
            });
        
    });

@ocarreterom
Copy link
Contributor

@jdnierth I think your changes are not following the UMD pattern.

@@ -6,11 +6,11 @@

(function(factory){
if (typeof define === 'function' && define.amd) {
define(['jquery'], factory);
define(['jquery'], factory(jQuery || window.jQuery || require('jquery')));
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ocarreterom , thanks for your comment but the UMD pattern is still in tact, the only change is to pass through variable declarations that can be different per environment. Happy to discuss if you have further feedback.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ocarreterom I´m using this plugin in an AMD environment and it fails with the error message described in the bugs description without this line.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test this line?
The second argument of define([dependecies], callback) should be a function (a callback like factory) to get the dependencies as params, not the result of the function factory().

https://requirejs.org/docs/api.html#defdep

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ocarreterom, I did within a custom application but I´ll decline this PR until I´ve made sure I rule out any other reason why I experienced this error. Thanks for your patience.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ocarreterom sorry I can`t decline this PR nor can I give it a label. In case you have more access permissions could you please put a label "on hold" on it? Many thanks in advance.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @vsn4ik

} else if (typeof exports === 'object') {
factory(require('jquery'));
factory(jQuery || window.jQuery || require('jquery'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jdnierth
Does this line not throw an error in a Node/Commonjs environment?

Copy link
Author

@jdnierth jdnierth Jun 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

facepalm you are correct it should throw an error in Node.js environment since there is no window object, hence reverted this line. Thanks for the nudge @ocarreterom !

} else {
factory(jQuery);
factory(jQuery || window.jQuery);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverted this line to former factory(jQuery)

@mohamedkofta
Copy link

Q A
Bug fix? yes
New feature? no
BC breaks? no
Related tickets mentioned #2362, #1484, #1196
License MIT

Description

  • Fixing bug 't.fn.datepicker is undefined' firing after minification.
  • Fixing bug by making check for jquery as a dependency more defensive within AMD check.
  • Setup within require.js with jquery exported (see project configuration further down)
  • jQuery version 2.2.1
  • Bootstrap Datepicker version: 1.8.0

Console.log screenshot

console-error

Project specific setup

HTML structure

 <div class="js-datepicker-container">
      <input type="text" class="form-control" id="dob" placeholder="Date Naisssance*(JJ/MM/AAAA)">
 </div>

RequireJS config excerpt

var require = {
    paths: {
        'jquery': 'Vendor/jquery-2.2.4',
        'bs-dropdown': 'Plugins/bootstrap/dropdown',
        'bs-datepicker': 'Plugins/bootstrap-datepicker/bootstrap-datepicker',
        'bs-datepicker-lang-fr': 'Plugins/bootstrap-datepicker/locales/bootstrap-datepicker.fr',
	...
    },
    shim: {
        'jquery': {exports: '$'},
        'bs-datepicker-lang-fr': [
            'jquery',
            'bs-datepicker',
        ]
    }
};

Custom module usage

define([
       	'jquery',
        'bs-datepicker-lang-fr'
    ],
    function search(
        $,
        bsDatepicker
    ) {
     
            var $datepickerContainer = $('.js-datepicker-container input');

            $datepickerContainer.datepicker({
                forceParse: false,
                format: 'dd/mm/yyyy'
            });
        
    });

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

Successfully merging this pull request may close these issues.

None yet

3 participants