Skip to content

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.

(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

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

@ocarreterom ocarreterom Jun 10, 2019

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 !

factory(jQuery || window.jQuery || require('jquery'));
} 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.

3 participants