search

sveltejs / svelte-loader

Webpack loader for svelte components.
MIT License
594 stars 73 forks source link

Validate error path for addDependency() (#197) #199

Closed wabain closed 2 years ago

wabain commented 2 years ago

Fixes #197, which is caused by undefined being passed to the loader addDependency method when handling errors. A Svelte CompileError doesn't have a file property, but it does have filename, so try either of those and call addDependency with the value if it is a string (and not equal to the path of the top-level resource being loaded).

In addition to updating tests, I've checked manually that error reporting and rebuilding after an error works in a webpack project that had been hitting ERR_INVALID_ARG_TYPE before this patch.

basuke commented 2 years ago

Why isn't this PR merged in? The crash is very annoying. I appreciate this merging-in pretty soon.

pngwn commented 2 years ago

Your potential appreciation of any 'pretty-soon' merging is duly noted. Thank you for informing us.

basuke commented 2 years ago

@pngwn I'm sorry if my comment made you feeling bad but I really do not get the reason why this PR is halting. Are there any issue or something unclear?

FYI: the crash won't happens when I don't use <script lang=ts>. The compilation error will be caught by the other error handler: https://github.com/sveltejs/svelte-loader/blob/79111a95f910dcdde7a8612334ee56c80653874e/index.js#L87-L89

pngwn commented 2 years ago

I'm sorry if my comment made you feeling bad but I really do not get the reason why this PR is halting. Are there any issue or something unclear?

No other reason that we are very busy. We'll get around to it as soon as we can.