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

Remove jQuery dependency #55

Closed
adamschwartz opened this issue May 4, 2014 · 18 comments
Closed

Remove jQuery dependency #55

adamschwartz opened this issue May 4, 2014 · 18 comments

Comments

@adamschwartz
Copy link
Contributor

Clearly peeps want this, also, cuz

@adamschwartz adamschwartz changed the title Remove jQuery support Remove jQuery dependency Jul 10, 2014
@ch3stnut
Copy link

Yes please!

@ngault
Copy link

ngault commented Oct 17, 2014

+1

4 similar comments
@hausofwong
Copy link

+1

@andylima
Copy link

+1

@mfrye-intuit
Copy link

+1

@tborychowski
Copy link

+1

@ericlenio
Copy link

I took a crack at using Zepto instead of jQuery. It works for my project, but I probably missed a few things. My general strategy: eliminate serializing the options into vex DOM object "data-vex" attributes, and instead keep them in a global hash object (optionsHash) since core Zepto does not duplicate $.data exactly (there is a plugin for it, but I wanted to avoid that); each vex DOM object instead just keeps track of its vex id.

diff --git a/js/vex.dialog.js b/js/vex.dialog.js
index 6d3b873..dbf3911 100644
--- a/js/vex.dialog.js
+++ b/js/vex.dialog.js
@@ -33,8 +33,10 @@
         type: 'button',
         className: 'vex-dialog-button-secondary',
         click: function($vexContent, event) {
-          $vexContent.data().vex.value = false;
-          return vex.close($vexContent.data().vex.id);
+          var id = $vexContent.data('vex-id');
+          var options = vex.getOptionsById(id);
+          options.value = false;
+          return vex.close(id);
         }
       }
     };
@@ -52,8 +54,10 @@
         $vexContent = $form.parent();
         event.preventDefault();
         event.stopPropagation();
-        $vexContent.data().vex.value = dialog.getFormValueOnSubmit($formToObject($form));
-        return vex.close($vexContent.data().vex.id);
+        var id = $vexContent.data('vex-id');
+        var options = vex.getOptionsById(id);
+        options.value = dialog.getFormValueOnSubmit($formToObject($form));
+        return vex.close(id);
       },
       focusFirstInput: true
     };
@@ -147,7 +151,7 @@
   } else if (typeof exports === 'object') {
     module.exports = vexDialogFactory(require('jquery'), require('./vex.js'));
   } else {
-    window.vex.dialog = vexDialogFactory(window.jQuery, window.vex);
+    window.vex.dialog = vexDialogFactory(typeof(jQuery)=="undefined" ? Zepto : jQuery, vex);
   }

 }).call(this);
diff --git a/js/vex.js b/js/vex.js
index 2b3884c..9e6d18f 100644
--- a/js/vex.js
+++ b/js/vex.js
@@ -16,6 +16,7 @@
     });
     return vex = {
       globalID: 1,
+      optionsHash: {},
       animationEndEvent: 'animationend webkitAnimationEnd mozAnimationEnd MSAnimationEnd oanimationend',
       baseClassNames: {
         vex: 'vex',
@@ -44,30 +45,33 @@
         options = $.extend({}, vex.defaultOptions, options);
         options.id = vex.globalID;
         vex.globalID += 1;
-        options.$vex = $('<div>').addClass(vex.baseClassNames.vex).addClass(options.className).css(options.css).data({
-          vex: options
-        });
-        options.$vexOverlay = $('<div>').addClass(vex.baseClassNames.overlay).addClass(options.overlayClassName).css(options.overlayCSS).data({
-          vex: options
-        });
+        vex.optionsHash[options.id] = options;
+        options.$vex = $('<div>')
+          .addClass(vex.baseClassNames.vex)
+          .addClass(options.className)
+          .css(options.css).data('vex-id',options.id);
+        options.$vexOverlay = $('<div>')
+          .addClass(vex.baseClassNames.overlay)
+          .addClass(options.overlayClassName)
+          .css(options.overlayCSS).data('vex-id',options.id);
         if (options.overlayClosesOnClick) {
           options.$vexOverlay.bind('click.vex', function(e) {
             if (e.target !== this) {
               return;
             }
-            return vex.close($(this).data().vex.id);
+            return vex.close(options.id);
           });
         }
         options.$vex.append(options.$vexOverlay);
-        options.$vexContent = $('<div>').addClass(vex.baseClassNames.content).addClass(options.contentClassName).css(options.contentCSS).append(options.content).data({
-          vex: options
-        });
+        options.$vexContent = $('<div>').addClass(vex.baseClassNames.content).addClass(options.contentClassName).css(options.contentCSS).append(options.content).data(
+          "vex-id", options.id
+        );
         options.$vex.append(options.$vexContent);
         if (options.showCloseButton) {
-          options.$closeButton = $('<div>').addClass(vex.baseClassNames.close).addClass(options.closeClassName).css(options.closeCSS).data({
-            vex: options
-          }).bind('click.vex', function() {
-            return vex.close($(this).data().vex.id);
+          options.$closeButton = $('<div>').addClass(vex.baseClassNames.close).addClass(options.closeClassName).css(options.closeCSS).data(
+            "vex-id", options.id
+          ).bind('click.vex', function() {
+            return vex.close(options.id);
           });
           options.$vexContent.append(options.$closeButton);
         }
@@ -85,11 +89,16 @@
         return "." + (baseClass.split(' ').join('.'));
       },
       getAllVexes: function() {
-        return $("." + vex.baseClassNames.vex + ":not(\"." + vex.baseClassNames.closing + "\") " + (vex.getSelectorFromBaseClass(vex.baseClassNames.content)));
+        return $("." + vex.baseClassNames.vex)
+         .not("." + vex.baseClassNames.closing)
+         .not(vex.getSelectorFromBaseClass(vex.baseClassNames.content));
+      },
+      getOptionsById: function(id) {
+        return vex.optionsHash[id];
       },
       getVexByID: function(id) {
         return vex.getAllVexes().filter(function() {
-          return $(this).data().vex.id === id;
+          return $(this).data('vex-id') === id;
         });
       },
       close: function(id) {
@@ -99,14 +108,14 @@
           if (!$lastVex.length) {
             return false;
           }
-          id = $lastVex.data().vex.id;
+          id = $lastVex.data('vex-id');
         }
         return vex.closeByID(id);
       },
       closeAll: function() {
         var ids;
         ids = vex.getAllVexes().map(function() {
-          return $(this).data().vex.id;
+          return $(this).data('vex-id');
         }).toArray();
         if (!(ids != null ? ids.length : void 0)) {
           return false;
@@ -117,19 +126,22 @@
         return true;
       },
       closeByID: function(id) {
-        var $vex, $vexContent, beforeClose, close, hasAnimation, options;
+        var options = vex.getOptionsById(id);
+        var $vex, $vexContent, beforeClose, close, hasAnimation;
         $vexContent = vex.getVexByID(id);
         if (!$vexContent.length) {
           return;
         }
-        $vex = $vexContent.data().vex.$vex;
-        options = $.extend({}, $vexContent.data().vex);
+        $vex = $vexContent.closest('.vex');
         beforeClose = function() {
           if (options.beforeClose) {
             return options.beforeClose($vexContent, options);
           }
         };
         close = function() {
+          if (id in vex.optionsHash) {
+            delete vex.optionsHash[id];
+          }
           $vexContent.trigger('vexClose', options);
           $vex.remove();
           $('body').trigger('vexAfterClose', options);
@@ -140,7 +152,7 @@
         hasAnimation = $vexContent.css('animationName') !== 'none' && $vexContent.css('animationDuration') !== '0s';
         if (animationEndSupport && hasAnimation) {
           if (beforeClose() !== false) {
-            $vex.unbind(vex.animationEndEvent).bind(vex.animationEndEvent, function() {
+            $vex.off(vex.animationEndEvent).bind(vex.animationEndEvent, function() {
               return close();
             }).addClass(vex.baseClassNames.closing);
           }
@@ -154,14 +166,15 @@
       closeByEscape: function() {
         var $lastVex, id, ids;
         ids = vex.getAllVexes().map(function() {
-          return $(this).data().vex.id;
+          return $(this).data('vex-id');
         }).toArray();
         if (!(ids != null ? ids.length : void 0)) {
           return false;
         }
         id = Math.max.apply(Math, ids);
         $lastVex = vex.getVexByID(id);
-        if ($lastVex.data().vex.escapeButtonCloses !== true) {
+        var options = vex.getOptionsById(id);
+        if (options.escapeButtonCloses !== true) {
           return false;
         }
         return vex.closeByID(id);
@@ -190,7 +203,7 @@
   } else if (typeof exports === 'object') {
     module.exports = vexFactory(require('jquery'));
   } else {
-    window.vex = vexFactory(jQuery);
+    window.vex = vexFactory(typeof(jQuery)=="undefined" ? Zepto : jQuery);
   }

 }).call(this);

@tborychowski
Copy link

@ericlenio I believe you're missing the point of this request.

It's remove jQuery dependency

and not:

It's remove jQuery dependency

😄

@ericlenio
Copy link

@tborychowski yeah OK I see what you mean. I made a 2nd attempt to do it in ericlenio@6e93c38. Notes about it:

  • this is my first attempt at coffeescript, I am sure I did some things "really ugly"
  • only limited testing in Chrome
  • I have zero experience with AMD and CommonJS, they are completely untested
  • I removed 3 apparently unused methods: showLoading, hideLoading, closeAll
  • I was not sure how to properly handle namespaced events, like click.vex, so I just used regular events
  • I'm really hoping one of you who knows coffeescript can review this, and ideally it can be merged back to this project

@markstos
Copy link
Contributor

There's a new vex2 project which removes the jQuery dependency. Take note though, the project is very new and there are some known bugs and backwards-compatibility issues still to address.

https://github.com/bbatliner/vex2

https://github.com/bbatliner/vex2-dialog/issues

@zackbloom
Copy link
Contributor

I wonder why @bbatliner elected to not make that a PR, I think removing jQuery would be great for this project.

@markstos
Copy link
Contributor

It's more of a rewrite than a one-issue fix. vex2 has other notable
changes:

  • written in JavaScript instead of CoffeeScript
  • modular plugin-system

I'm also suggesting that as long as the module is new that vex2 make a
breaking change to make the use "secure by default" by treating the
"messages" passed through as strings rather than raw HTML. HTML could still
be used if it was passed through a new 'unsafeMessageproperty, which would a clue to the developer or QA person that the string needs to be escaped. This change really only applies to thedialog` plugin, not the
core. I've published a fork of the plugin that works like that:

https://github.com/RideAmigosCorp/vex2-safe-dialog

The vex dialog plugin claimed to a "drop in replacement" for the standard
alert, confirm and prompt messages, but vex made a major change from
the built-ins as far as security is concerned. The original methods didn't
allow HTML through, but vex does, leaving it up to developers to remember
to always properly escape untrusted values or have XSS vulnerabilities.
XSS holes are unlikely to be noticed unless you are specifically testing
for them in the current design.

However, if vex made HTML escaping the /default/, developers are definitely
going to notice if they intended some HTML to render and got escaped HTML
instead. Then they'll realize they need to use the 'unsafeMessage' property
pass raw HTML through. The property name should then hopefully be enough a
reminder that if raw HTML is going to be passed through, property escaping
or sanitizing untrusted data needs to happen.

Ten years ago HTML templating engines defaulted to allowing raw HTML
through variables by default.

Templating engines released in the last few years generally switch to the
safer default of HTML-escaping variables.

Related tools like vex would be doing a community service to provide
security protection by default, just the built-ins vex was intended to
replace did.

On Fri, Jul 15, 2016 at 2:54 PM, Zack Bloom notifications@github.com
wrote:

I wonder why @bbatliner https://github.com/bbatliner elected to not
make that a PR, I think removing jQuery would be great for this project.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#55 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AABk5V8LhwpDOuuNGlSJwA5X9dOhyKj8ks5qV9dTgaJpZM4B3vNs
.

Mark Stosberg
Senior Systems Engineer
RideAmigos

@markstos
Copy link
Contributor

The vex2 author did ask for feedback from HubSpot on Twitter, but has
gotten no reply from them in that medium so far:

https://twitter.com/thebatliner/status/752514232646635520

On Fri, Jul 15, 2016 at 3:44 PM, Mark Stosberg mark@rideamigos.com wrote:

It's more of a rewrite than a one-issue fix. vex2 has other notable
changes:

  • written in JavaScript instead of CoffeeScript
  • modular plugin-system

I'm also suggesting that as long as the module is new that vex2 make a
breaking change to make the use "secure by default" by treating the
"messages" passed through as strings rather than raw HTML. HTML could still
be used if it was passed through a new 'unsafeMessageproperty, which would a clue to the developer or QA person that the string needs to be escaped. This change really only applies to thedialog` plugin, not the
core. I've published a fork of the plugin that works like that:

https://github.com/RideAmigosCorp/vex2-safe-dialog

The vex dialog plugin claimed to a "drop in replacement" for the standard
alert, confirm and prompt messages, but vex made a major change from
the built-ins as far as security is concerned. The original methods didn't
allow HTML through, but vex does, leaving it up to developers to remember
to always properly escape untrusted values or have XSS vulnerabilities.
XSS holes are unlikely to be noticed unless you are specifically testing
for them in the current design.

However, if vex made HTML escaping the /default/, developers are
definitely going to notice if they intended some HTML to render and got
escaped HTML instead. Then they'll realize they need to use the
'unsafeMessage' property pass raw HTML through. The property name should
then hopefully be enough a reminder that if raw HTML is going to be passed
through, property escaping or sanitizing untrusted data needs to happen.

Ten years ago HTML templating engines defaulted to allowing raw HTML
through variables by default.

Templating engines released in the last few years generally switch to the
safer default of HTML-escaping variables.

Related tools like vex would be doing a community service to provide
security protection by default, just the built-ins vex was intended to
replace did.

On Fri, Jul 15, 2016 at 2:54 PM, Zack Bloom notifications@github.com
wrote:

I wonder why @bbatliner https://github.com/bbatliner elected to not
make that a PR, I think removing jQuery would be great for this project.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#55 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AABk5V8LhwpDOuuNGlSJwA5X9dOhyKj8ks5qV9dTgaJpZM4B3vNs
.

Mark Stosberg
Senior Systems Engineer
RideAmigos

Mark Stosberg
Senior Systems Engineer
RideAmigos

@adamschwartz
Copy link
Contributor Author

adamschwartz commented Jul 15, 2016

For the record, I don’t think I’d have a problem accepting most if not all of those changes in a PR. Breaking changes are not a deal breaker. We would just slap a 3.0 and let people know how to update their method calls. The library has to change with best practices, so we should do whatever continues to keep vex as useful/beneficial to the community as possible. That being said, if the authors of vex2 have plans to take vex in a different direction entirely, feel free. But nothing I’ve heard or seen so far suggests it’s too much of a departure to be merged back in, caveats above.

@markstos
Copy link
Contributor

Thanks for the reply, @adamschwartz . vex2 author mentioned being busy with work doing the week, so perhaps he can join the conversation this weekend.

@bbatliner
Copy link
Collaborator

Hi everyone - I'm the maintainer of vex2. My main goals when rewriting vex were:

  1. Remove jQuery and try to be dependency-free or as lightweight as possible
  2. Simplify the library by relying on more "JavaScript-y" patterns like closures, not storing state in the DOM, etc.
  3. Keep as much backwards compatibility with vex and browser support as possible

I agree with @markstos's safe-by-default approach and would certainly want to incorporate his changes into vex2 before looking to merge back to vex. @adamschwartz I also see talk of moving to a modern build tool (Gulp) and using ES6, which I would be happy to include with vex2 since I've already moved away from CoffeeScript.

My thought is that we can work together to prepare vex2 to be merged back into vex as a PR, as you suggested. Upgrade guides, API docs, and bugfixes/compatibility issues are what's needed to get to that point. What do you think?

@adamschwartz
Copy link
Contributor Author

@bbatliner that all sounds good to me. Thanks so much for working on this. I think the community will benefit greatly from the improvements and changes your talking about.

@bbatliner
Copy link
Collaborator

bbatliner commented Aug 9, 2016

Woot! We made it - vex2 has been merged upstream. Thanks for the support everyone.

Edit: The latest major version release of vex is currently in beta. Install it with npm i vex-js@3.0.0-beta.1 vex-dialog (or download the files from the repo) and check the documentation for breaking changes.

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

No branches or pull requests