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

babel plugin: undefined in className #818

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@slorber

slorber commented Aug 22, 2018

What: try to fix the babel plugin so that it filters out undefined from final className

Why: it's bad to have class="xxx undefined" in production

How: by modifying the code

Checklist:

  • Documentation N/A
  • Tests N/A
  • Code complete N/A

I tried to add inline cases/snapshots the problematic cases:

  'undefined className': {
    code: `(<div className={undefined} css={\`color: $\{color};\`}></div>)`
  },
  'undefined const className': {
    code: `const cls = undefined; (<div className={cls} css={\`color: $\{color};\`}></div>)`
  },
  '"undefined" className': {
    code: `(<div className="undefined" css={\`color: $\{color};\`}></div>)`
  },

As of now, it's not perfectly working, I don't know how to avoid the "x" + "undefined" in the 3nd case.

@mitchellhamilton any idea? I gave it a try but I don't really know what I'm doing here

@slorber slorber changed the title from Hi, to babel plugin: undefined in className Aug 22, 2018

@codecov

This comment has been minimized.

codecov bot commented Aug 22, 2018

Codecov Report

Merging #818 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted Files Coverage Δ
packages/babel-plugin-emotion/src/css-prop.js 100% <100%> (ø) ⬆️
@mitchellhamilton

This comment has been minimized.

Member

mitchellhamilton commented Sep 15, 2018

You would have to add a ternary and check at runtime for undefined.

Just FYI, this will be fixed in v10.

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