trullock / NUglify

NUglify is a HTML, JavaScript and CSS minification Library for .NET (fork of AjaxMin + new features)
Other
398 stars 79 forks source link

Duplicate property check incorrectly marks function properties as having name "NUglify.JavaScript.Syntax.FunctionObjectmethod" and triggers a false duplicate property error #264

Closed rwasef1830 closed 3 years ago

rwasef1830 commented 3 years ago

Hello, Attempting to minify a bundle containing bootstrap.bundle.js produces the following errors:

wwwroot\dist\site.min.js: Line 12421, Column: 9: Strict-mode does not allow duplicate property names: ...Manipulator.getDataAttributes(this)
wwwroot\dist\site.min.js: Line 11879, Column: 5: Strict-mode does not allow duplicate property names: removeDataAttribute(element, key) {
      element.removeAttribute(`data-bs-${normalizeDataKey(key)}`);
    }
wwwroot\dist\site.min.js: Line 11883, Column: 5: Strict-mode does not allow duplicate property names: getDataAttributes(element) {
      if (!element) {
        return {};
      }

      const attributes = {};
      Object.keys(element.dataset).filter(key => key.startsWith('bs')).forEach(key => {
        let pureKey = key.replace(/^bs/, '');
        pureKey = pureKey.charAt(0).toLowerCase() + pureKey.slice(1, pureKey.length);
        attributes[pureKey] = normalizeData(element.dataset[key]);
      });
      return attributes;
    }
wwwroot\dist\site.min.js: Line 11897, Column: 5: Strict-mode does not allow duplicate property names: getDataAttribute(element, key) {
      return normalizeData(element.getAttribute(`data-bs-${normalizeDataKey(key)}`));
    }
wwwroot\dist\site.min.js: Line 11901, Column: 5: Strict-mode does not allow duplicate property names: offset(element) {
      const rect = element.getBoundingClientRect();
      return {
        top: rect.top + document.body.scrollTop,
        left: rect.left + document.body.scrollLeft
      };
    }
wwwroot\dist\site.min.js: Line 11909, Column: 5: Strict-mode does not allow duplicate property names: position(element) {
      return {
        top: element.offsetTop,
        left: element.offsetLeft
      };
    }
wwwroot\dist\site.min.js: Line 11559, Column: 8: Setter methods must have a single formal argument: (element, key, instance)
wwwroot\dist\site.min.js: Line 11442, Column: 5: Strict-mode does not allow duplicate property names: one(element, event, handler, delegationFn) {
      addHandler(element, event, handler, delegationFn, true);
    }
wwwroot\dist\site.min.js: Line 11446, Column: 5: Strict-mode does not allow duplicate property names: off(element, originalTypeEvent, handler, delegationFn) {
      if (typeof originalTypeEvent !== 'string' || !element) {
        return;
      }

      const [delegation, originalHandler, typeEvent] = normalizeParams(originalTypeEvent, handler, delegationFn);
      const inNamespace = typeEvent !== originalTypeEvent;
      const events = getEvent(element);
      const isNamespace = originalTypeEvent.startsWith('.');

      if (typeof originalHandler !== 'undefined') {
        // Simplest case: handler is passed, remove that listener ONLY.
        if (!events || !events[typeEvent]) {
          return;
        }

        removeHandler(element, events, typeEvent, originalHandler, delegation ? handler : null);
        return;
      }

      if (isNamespace) {
        Object.keys(events).forEach(elementEvent => {
          removeNamespacedHandlers(element, events, elementEvent, originalTypeEvent.slice(1));
        });
      }

      const storeElementEvent = events[typeEvent] || {};
      Object.keys(storeElementEvent).forEach(keyHandlers => {
        const handlerKey = keyHandlers.replace(stripUidRegex, '');

        if (!inNamespace || originalTypeEvent.includes(handlerKey)) {
          const event = storeElementEvent[keyHandlers];
          removeHandler(element, events, typeEvent, event.originalHandler, event.delegationSelector);
        }
      });
    }
wwwroot\dist\site.min.js: Line 11483, Column: 5: Strict-mode does not allow duplicate property names: trigger(element, event, args) {
      if (typeof event !== 'string' || !element) {
        return null;
      }

      const $ = getjQuery();
      const typeEvent = getTypeEvent(event);
      const inNamespace = event !== typeEvent;
      const isNative = nativeEvents.has(typeEvent);
      let jQueryEvent;
      let bubbles = true;
      let nativeDispatch = true;
      let defaultPrevented = false;
      let evt = null;

      if (inNamespace && $) {
        jQueryEvent = $.Event(event, args);
        $(element).trigger(jQueryEvent);
        bubbles = !jQueryEvent.isPropagationStopped();
        nativeDispatch = !jQueryEvent.isImmediatePropagationStopped();
        defaultPrevented = jQueryEvent.isDefaultPrevented();
      }

      if (isNative) {
        evt = document.createEvent('HTMLEvents');
        evt.initEvent(typeEvent, bubbles, true);
      } else {
        evt = new CustomEvent(event, {
          bubbles,
          cancelable: true
        });
      } // merge custom information in our event

      if (typeof args !== 'undefined') {
        Object.keys(args).forEach(key => {
          Object.defineProperty(evt, key, {
            get() {
              return args[key];
            }

          });
        });
      }

      if (defaultPrevented) {
        evt.preventDefault();
      }

      if (nativeDispatch) {
        element.dispatchEvent(evt);
      }

      if (evt.defaultPrevented && typeof jQueryEvent !== 'undefined') {
        jQueryEvent.preventDefault();
      }

      return evt;
    }
wwwroot\dist\site.min.js: Line 10903, Column: 5: Strict-mode does not allow duplicate property names: findOne(selector, element = document.documentElement) {
      return Element.prototype.querySelector.call(element, selector);
    }
wwwroot\dist\site.min.js: Line 10907, Column: 5: Strict-mode does not allow duplicate property names: children(element, selector) {
      return [].concat(...element.children).filter(child => child.matches(selector));
    }
wwwroot\dist\site.min.js: Line 10911, Column: 5: Strict-mode does not allow duplicate property names: parents(element, selector) {
      const parents = [];
      let ancestor = element.parentNode;

      while (ancestor && ancestor.nodeType === Node.ELEMENT_NODE && ancestor.nodeType !== NODE_TEXT) {
        if (ancestor.matches(selector)) {
          parents.push(ancestor);
        }

        ancestor = ancestor.parentNode;
      }

      return parents;
    }
wwwroot\dist\site.min.js: Line 10926, Column: 5: Strict-mode does not allow duplicate property names: prev(element, selector) {
      let previous = element.previousElementSibling;

      while (previous) {
        if (previous.matches(selector)) {
          return [previous];
        }

        previous = previous.previousElementSibling;
      }

      return [];
    }
wwwroot\dist\site.min.js: Line 10940, Column: 5: Strict-mode does not allow duplicate property names: next(element, selector) {
      let next = element.nextElementSibling;

      while (next) {
        if (next.matches(selector)) {
          return [next];
        }

        next = next.nextElementSibling;
      }

      return [];
    }

At the moment I don't know if the duplicate property error is true or not, but in general ECMAScript 6 strict mode was updated to allow duplicate properties, see: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Object_initializer

Duplicate property names When using the same name for your properties, the second property will overwrite the first.

let a = {x: 1, x: 2} console.log(a) // {x: 2} In ECMAScript 5 strict mode code, duplicate property names were considered a SyntaxError. With the introduction of computed property names making duplication possible at runtime, ECMAScript 2015 has removed this restriction.

rwasef1830 commented 3 years ago

Minimized test case that triggers the false error:

'use strict';

const SomeClass = {
    property1() {
    },

    property2() {
    }
};
rwasef1830 commented 3 years ago

The problem is here: https://github.com/trullock/NUglify/blob/a87b7d1493bb919c94916e5d9fc23d69a17c35ff/src/NUglify/JavaScript/Visitors/AnalyzeNodeVisitor.cs#L3365

property.Name ends up null which seems wrong, it should be the name of the function I think.

trullock commented 3 years ago

The line youve found seems to show the symptom not the cause. The property should have a name by this point.

trullock commented 3 years ago

JSParser Line 4924 is where the issue stems from