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

SyntaxError: missing formal parameter (Firefox) #4463

Closed
soullivaneuh opened this issue Feb 25, 2020 · 4 comments · Fixed by #4475
Closed

SyntaxError: missing formal parameter (Firefox) #4463

soullivaneuh opened this issue Feb 25, 2020 · 4 comments · Fixed by #4475
Labels

Comments

@soullivaneuh
Copy link

Describe the bug

The svelte title is replaced by 500 and I have this error on the console:

SyntaxError: missing formal parameter

Logs
Please include browser console and server logs around the time this bug occurred.

To Reproduce
To help us help you, if you've found a bug please consider the following:

  • Create a svelte template
  • Require the tailwind default config (this produce the bug):
const {
  colors, boxShadow, fontFamily, fontSize, opacity,
} = require('tailwindcss/defaultTheme');

You can also checkout this public MR: https://gitlab.com/nexylan/design/-/merge_requests/42

Expected behavior
Working without any error.

Severity
Happen on dev env only for a demo project, so not a big deal to me.

Could be problematic for other, especially with the 500 title issue.

Additional context

This happens only on v3.19.0 and v3.19.1, maybe a BC break?

I also tried a Tailwind downgrade to the old version I used before, the error still happen with Svelte 3.19.

@Conduitry
Copy link
Member

Part of this is definitely webpack doing something peculiar, but I think what Svelte's doing that might be wrong is including the unknown global require in the object returned by $capture_state. Webpack is then apparently doing something weird with the bare require when it's not immediately called on anything.

@rixo Do you have thoughts about making $capture_state not include unknown globals? That would seem to make sense to me.

@Conduitry Conduitry added the awaiting submitter needs a reproduction, or clarification label Feb 25, 2020
@rixo
Copy link
Contributor

rixo commented Feb 26, 2020

@Conduitry How do you qualify "unknown" globals? Anyway, I think we should exclude all of them from $capture_state.

For HMR it's irrelevant, but I guess that'd be weird to end up with things like window, or requestAnimationFrame, or require in the inspect view of dev tools. There might, in some cases, be a micro interest in viewing user's own globals, but even that could be weird too, depending on the variable. And that would need some kind of white list, which makes it prohibitively complicated to implement IMO, considering these variables are very easy to inspect in the console, since they're global.

So yeah, I think we should definitely remove globals from $capture_state. It would also protect people from this Webpack bug. I'll send a PR for this.

@RedHatter Can you confirm that globals are not needed for dev tools? Or did I miss something?

@soullivaneuh I think you shouldn't be using require anymore in frontend code nowadays (and especially in a Svelte component)... Replace it with the following, and your bug will be fixed with 3.19.1 right away:

import config from "../../tailwind.config.js";

(And for what it's worth, the whole client side component is crashed... What you get is just the SSR'd page.)

@RedHatter
Copy link
Contributor

Yeah, excluding globals from $capture_state is fine and probably preferable.

@Conduitry
Copy link
Member

Sorry, yeah, 'unknown' was a bit of a loaded term. I was really just asking about all variables that aren't declared (possibly implicitly) or imported in the component - and it sounds like there's agreement that these should be excluded when capturing the state. I don't think there should be a whitelist of globals that should still be included anyway in the captured state.

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.

4 participants