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

error when referencing an event with dashes ("-") and no event handler #2923

Closed
opensas opened this issue Jun 1, 2019 · 5 comments · Fixed by #3111
Closed

error when referencing an event with dashes ("-") and no event handler #2923

opensas opened this issue Jun 1, 2019 · 5 comments · Fixed by #3111
Labels

Comments

@opensas
Copy link
Contributor

opensas commented Jun 1, 2019

When referencing an event with dashes in it's name and no event handler it gives the following error:

[!] Error: Unexpected token (Note that you need plugins to import files that are not JavaScript)

Repl with the error: https://svelte.dev/repl/ec066729fa684c90a2fa16e6eb730ed0?version=3.4.4

Example:

<script>
	let name = 'world';
</script>

<h1 on:click-now={() => alert('hi')}>Hello {name}!</h1>

<h1 on:click-now>Hello {name}!</h1>

The first h1 works ok, the seconds raises the error

Stack trace

[2019-06-01 07:11:25] waiting for changes...
rollup v1.13.0
bundles src/main.js → public/bundle.js...
(!) Error when using sourcemap for reporting an error: Can't resolve original location of error.
src/App.svelte: (83:15)
[!] Error: Unexpected token (Note that you need plugins to import files that are not JavaScript)
src/App.svelte (83:15)
Error: Unexpected token (Note that you need plugins to import files that are not JavaScript)
at error (/media/data/sas/devel/apps/svelte/udemy/section_6_custom_events/custom_events/node_modules/.registry.npmjs.org/rollup/1.13.0/node_modules/rollup/dist/rollup.js:9241:30)
at Module.error (/media/data/sas/devel/apps/svelte/udemy/section_6_custom_events/custom_events/node_modules/.registry.npmjs.org/rollup/1.13.0/node_modules/rollup/dist/rollup.js:13085:9)
at tryParse (/media/data/sas/devel/apps/svelte/udemy/section_6_custom_events/custom_events/node_modules/.registry.npmjs.org/rollup/1.13.0/node_modules/rollup/dist/rollup.js:13001:16)
at Module.setSource (/media/data/sas/devel/apps/svelte/udemy/section_6_custom_events/custom_events/node_modules/.registry.npmjs.org/rollup/1.13.0/node_modules/rollup/dist/rollup.js:13304:33)
at Promise.resolve.catch.then.then.then (/media/data/sas/devel/apps/svelte/udemy/section_6_custom_events/custom_events/node_modules/.registry.npmjs.org/rollup/1.13.0/node_modules/rollup/dist/rollup.js:16027:20)

@Conduitry Conduitry added the bug label Jun 3, 2019
@Conduitry
Copy link
Member

The automatic event handler that Svelte writes which simply bubbles the event needs to have its name correctly sanitized like the other event handler already does.

@Conduitry
Copy link
Member

diff --git a/src/compiler/compile/nodes/EventHandler.ts b/src/compiler/compile/nodes/EventHandler.ts
index dab60858d..20ac44997 100644
--- a/src/compiler/compile/nodes/EventHandler.ts
+++ b/src/compiler/compile/nodes/EventHandler.ts
@@ -2,6 +2,7 @@ import Node from './shared/Node';
 import Expression from './shared/Expression';
 import Component from '../Component';
 import deindent from '../utils/deindent';
+import { sanitize } from '../../utils/names';
 import Block from '../render-dom/Block';
 
 export default class EventHandler extends Node {
@@ -41,7 +42,7 @@ export default class EventHandler extends Node {
 				}
 			}
 		} else {
-			const name = component.get_unique_name(`${this.name}_handler`);
+			const name = component.get_unique_name(`${sanitize(this.name)}_handler`);
 
 			component.add_var({
 				name,

@opensas
Copy link
Contributor Author

opensas commented Jun 4, 2019

great, I'm giving my first steps with npm link, I'll see how I can give your fix a try

BTW, it would be great for newbies (like me) to have a detailed guide covering how to play with svelte's source code, like cloning and using local svelte on your workstation (from what I understood you have to use npm link), creating a failing unit test, fixing the problem to make it pass, and issuing a pull request.

This is a good beginning, but I guess a more detailed guide would help many contributors give their first steps

@opensas
Copy link
Contributor Author

opensas commented Jun 4, 2019

alright, I could make it work using npm link and I can confirm that the little example I posted here works ok with the proposed fix

@opensas
Copy link
Contributor Author

opensas commented Jun 4, 2019

If I'm not mistaken, the unit test should be something like what it is at: /svelte/compiler/svelte/test/parser/samples/event-handler/

How can I generate the output.json so that it can be latter compared with _actual.json???

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants