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

Consider adding a warnings for incorrect tagged template literal usage in development #352

Closed
cesarandreu opened this issue Jan 4, 2017 · 9 comments

Comments

@cesarandreu
Copy link

I was adding styled-components to an app and global styles weren't showing up. There weren't any errors and it wasn't immediately obvious what I was doing wrong. It turned out I was using injectGlobal wrong:

// wrong: calling injectGlobal as a function. oops.
injectGlobal(`html { color: red; }`)

// correct: using a tagged template literal
injectGlobal`html { color: red; }`

The first argument of tagged template literals includes a raw property. Adding a check in development to prints out an error when strings.raw is missing would help catch this kind of mistake. I'm not sure how frequently this happens, but considering the relative rarity of tagged template literal usage, I figure it wouldn't hurt.

if (process.env.NODE_ENV !== 'production' && !('raw' in strings)) {
  console.error('...')
}
@vdanchenkov
Copy link
Member

vdanchenkov commented Jan 4, 2017

I would suggest to check it slightly different way. injectGlobal and other helpers expect array in first argument. So, let's check for it.

Something like.

if (process.env.NODE_ENV !== 'production' && !(Array.isArray(strings))) {
  console.error('...')
}

To clarify:

injectGlobal`html { color: red; }`
// works the same way as 
injectGlobal([ `html { color: red; }` ])
// (If there is no interpolations!!!)

On the other hand, we could support incorrect usage?

injectGlobal(`html { color: red; }`) 
// will be equivalent to
injectGlobal([ `html { color: red; }`])
// or 
injectGlobal`html { color: red; }`

@mxstbr
Copy link
Member

mxstbr commented Feb 4, 2017

This seems reasonable. Whatever we can do to avoid people adding bugs to their apps 👍

@iamsolankiamit
Copy link

I'll take this one. Just to clarify, we want to allow incorrect usage or show error when incorrectly used?

@mxstbr
Copy link
Member

mxstbr commented Jul 18, 2017

Introducing an error now would be a breaking change, so let's just log a warning?

maksugr pushed a commit to maksugr/styled-components that referenced this issue Oct 2, 2017
maksugr pushed a commit to maksugr/styled-components that referenced this issue Oct 3, 2017
@beautyfree
Copy link

Is it still open?

@chengjianhua
Copy link

@maksugr Could you make a PR for your completed commits?

@maksugr
Copy link

maksugr commented May 5, 2018

@chengjianhua it is right here

@chengjianhua
Copy link

@maksugr 🤣 So this issue can be closed now?

@quantizor
Copy link
Contributor

Yup

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

No branches or pull requests

8 participants