fix(event): add warns for events that start with "on"

ionic-team · Oct 15, 2019 · 1fa6636 · 1fa6636
1 parent dd58fd2
commit 1fa6636
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 9 deletions.
diff --git a/src/compiler/transformers/decorators-to-static/event-decorator.ts b/src/compiler/transformers/decorators-to-static/event-decorator.ts
@@ -87,15 +87,28 @@ const validateEventName = (config: d.Config, diagnostics: d.Diagnostic[], node:
       `Please lowercase the first character for the event to best work with all listeners.`
     ].join('');
     augmentDiagnosticWithNode(config, diagnostic, node);
+    return;
   }
 
-  if (DOM_EVENT_NAMES.has(eventName)) {
+  if (/^on[A-Z]/.test(eventName)) {
+    const warn = buildWarn(diagnostics);
+    warn.messageText = `Events decorated with @Event() should describe the actual DOM event name, not the handler. In other words "${eventName}" would be better named as "${suggestEventName(eventName)}".`;
+    augmentDiagnosticWithNode(config, warn, node);
+    return;
+  }
+
+  if (DOM_EVENT_NAMES.has(eventName.toLowerCase())) {
     const diagnostic = buildWarn(diagnostics);
     diagnostic.messageText = `The event name conflicts with the "${eventName}" native DOM event name.`;
     augmentDiagnosticWithNode(config, diagnostic, node);
+    return;
   }
 };
 
+function suggestEventName(onEvent: string) {
+  return onEvent[2].toLowerCase() + onEvent.slice(3);
+}
+
 const DOM_EVENT_NAMES: Set<string> = new Set([
   'CheckboxStateChange',
   'DOMContentLoaded',
@@ -357,4 +370,4 @@ const DOM_EVENT_NAMES: Set<string> = new Set([
   'vrdisplaypresentchange',
   'waiting',
   'wheel',
-]);
+].map(e => e.toLowerCase()));
diff --git a/src/compiler/transformers/decorators-to-static/prop-decorator.ts b/src/compiler/transformers/decorators-to-static/prop-decorator.ts
@@ -58,7 +58,13 @@ const parsePropDecorator = (config: d.Config, diagnostics: d.Diagnostic[], typeC
     augmentDiagnosticWithNode(config, err, prop.modifiers[0]);
   }
 
-  validatePublicName(config, diagnostics, propName, '@Prop()', 'prop', prop.name);
+  if (/^on(-|[A-Z])/.test(propName)) {
+    const warn = buildWarn(diagnostics);
+    warn.messageText = `The @Prop() name "${propName}" looks like an event. Please use the "@Event()" decorator to expose events instead, not properties or methods.`;
+    augmentDiagnosticWithNode(config, warn, prop.name);
+  } else {
+    validatePublicName(config, diagnostics, propName, '@Prop()', 'prop', prop.name);
+  }
 
   const symbol = typeChecker.getSymbolAtLocation(prop.name);
   const type = typeChecker.getTypeAtLocation(prop);

diff --git a/src/compiler/transformers/reserved-public-members.ts b/src/compiler/transformers/reserved-public-members.ts
@@ -4,12 +4,6 @@ import ts from 'typescript';
 
 
 export const validatePublicName = (config: d.Config, diagnostics: d.Diagnostic[], memberName: string, decorator: string, memberType: string, node: ts.Node) => {
-  if (/^on(-|[A-Z])/.test(memberName)) {
-    const warn = buildWarn(diagnostics);
-    warn.messageText = `The ${decorator} name "${memberName}" looks like an event. Please use the "@Event()" decorator to expose events instead, not properties or methods.`;
-    augmentDiagnosticWithNode(config, warn, node);
-    return;
-  }
   if (RESERVED_PUBLIC_MEMBERS.has(memberName.toLowerCase())) {
     const warn = buildWarn(diagnostics);
     warn.messageText = [