Skip to content

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

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.

Sign up for GitHub

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

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

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
45a4d58
fix (#1484 fixing 't.fn.datepicker is undefined' when loading bootstr…
Apr 24, 2019
a9f104e
fix (Added semicolong to fix failing travis build)
Apr 30, 2019
d2ea68b
feat (Committed files after running grunt test and grunt dist-js)
Apr 30, 2019
6c1821a
fix (Fixed wrong amend on commonJS case and only kept validated AMD s…
Jun 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions dist/js/bootstrap-datepicker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

@ocarreterom ocarreterom Jun 10, 2019 •
edited
Loading

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 •
edited
Loading

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)

}
}(function($, undefined){
function UTCDate(){
Expand Down Expand Up @@ -89,6 +89,10 @@

var Datepicker = function(element, options){
$.data(element, 'datepicker', this);

this._events = [];
this._secondaryEvents = [];

this._process_options(options);

this.dates = new DateArray();
Expand All @@ -98,7 +102,7 @@
this.element = $(element);
this.isInput = this.element.is('input');
this.inputField = this.isInput ? this.element : this.element.find('input');
this.component = this.element.hasClass('date') ? this.element.find('.add-on, .input-group-addon, .btn') : false;
this.component = this.element.hasClass('date') ? this.element.find('.add-on, .input-group-addon, .input-group-append, .input-group-prepend, .btn') : false;
if (this.component && this.component.length === 0)
this.component = false;
this.isInline = !this.component && this.element.is('div');
Expand Down Expand Up @@ -308,8 +312,6 @@
o.defaultViewDate = UTCToday();
}
},
_events: [],
_secondaryEvents: [],
_applyEvents: function(evs){
for (var i=0, el, ch, ev; i < evs.length; i++){
el = evs[i][0];
Expand Down Expand Up @@ -465,7 +467,7 @@
},

show: function(){
if (this.inputField.prop('disabled') || (this.inputField.prop('readonly') && this.o.enableOnReadonly === false))
if (this.inputField.is(':disabled') || (this.inputField.prop('readonly') && this.o.enableOnReadonly === false))
return;
if (!this.isInline)
this.picker.appendTo(this.o.container);
Expand Down
Loading