Allow ES6 class attrs in styled-component #1751

valerybugakov · 2018-05-22T15:03:27Z

A small fix to allow passing of the ES6 classes using .attrs()
I was trying to pass component prop to redux-form Field component, but got the error message:

Uncaught TypeError: Class constructor TextInput cannot be invoked without 'new'

It was mistakenly treated like a prop-factory.

quantizor · 2018-05-24T15:01:16Z

src/utils/isClass.js

+// @flow
+
+export default function isClass(target: any) {
+  return typeof target === 'function' && /^\s*class\s+/.test(target.toString())


I don't think this will work for transpiled classes?

Right, forgot about that and according to babel/babel#5640 it's not possible right now.

quantizor · 2018-05-25T05:27:03Z

src/models/StyledComponent.js

@@ -83,7 +84,8 @@ export default (ComponentStyle: Function, constructWithOptions: Function) => {
      this.attrs = Object.keys(attrs).reduce((acc, key) => {
        const attr = attrs[key]
        // eslint-disable-next-line no-param-reassign
-        acc[key] = typeof attr === 'function' ? attr(context) : attr
+        acc[key] =
+          typeof attr === 'function' && !isClass(attr) ? attr(context) : attr


Instead let's use one of the methods from the react-is package. I think it'll work better.

It seems that it's not possible too. With this package we can only check if attr is a valid element type for React but it will accept functions too.

Also: facebook/react#12578

Well if we’re just concerned with classes, can we simply check if React.Component is in the inheritance chain for the function?

Yeah, it's better than nothing. Will update PR.

valerybugakov · 2018-05-25T08:21:30Z

@probablyup PR updated

* master: Update styled-components.d.ts jest 23 v3.3.0 sort imports, use the new displayName util migrate displayName generation to a util update changelog for pull request 1753 support x attributes suppress false positive flow errors add the same for StyledNativeComponent fix syntax issue in eslintrc fix typo consoleWarn -> nativeWarn hoist non react & sc statics Fix flow Set testURL to silence JSDOM relative url error Upgrade Jest to 22

quantizor · 2018-05-29T03:33:27Z

src/models/StyledNativeComponent.js

@@ -42,8 +43,12 @@ export default (constructWithOptions: Function, InlineStyle: Function) => {

      this.attrs = Object.keys(attrs).reduce((acc, key) => {
        const attr = attrs[key]
+        const extendsReactComponent = hasInInheritanceChain(attr, Component)


let's inline this, no need to do the work unless attr is confirmed to be a function

quantizor

LGTM aside from the one request

* master: just only run lint on travis update eslint and fix new linting errors use a newer node on appveyor bump flow-bin version to 0.73.0 add changelog entry for styled-components#1748 remove lodash flow-typed defs chore: upgrade flow-bin to latest

valerybugakov · 2018-05-30T13:13:45Z

I haven't changed anything related to types in this PR. Flow should be happy as it is in master branch.
@probablyup do you have an idea why it might fail?

quantizor · 2018-05-30T14:55:10Z

@valerybugakov I think if you merge from master it should go away - did a lot of CI changes last night. Also, please make sure that test-results.json stays deleted from git.

* master: fix bundlesize param name adjust changelog to reflect the new PR number remove unactionable danger message run prettier on dangerfile remove test-results.json from source control update danger, travis & appveyor configs, bundlesize

valerybugakov · 2018-05-30T15:58:30Z

@probablyup deleted test-results.json and merged master. Still getting some errors from flow.

quantizor · 2018-05-30T16:05:03Z

Dang, ok. If you can't figure out what flow wants you to add a type for, feel free to throw a // $FlowFixMe above the offending line.

valerybugakov · 2018-05-30T17:41:07Z

Yeah, flow is pretty mysterious sometimes, bypassed errors for now.

quantizor · 2018-06-01T05:39:09Z

@valerybugakov try merging from master - I turned off danger for the moment so travis should stop being cranky

since travis is not allowing encrypted variables to be available to forks, it makes running danger impossible for most PRs I'm going to look into alternate services making use of the new Github Apps API that don't have this restriction.

valerybugakov · 2018-06-01T17:10:20Z

@probablyup let's merge until new conflicts have arrived :P

quantizor · 2018-06-01T17:15:24Z

ha no doubt

mxstbr · 2018-06-01T17:15:49Z

Thank you so much for helping us improve styled-components! Based on our Community Guidelines every person that has a PR of any kind merged is offered an invitation to the styled-components organization—that is you right now!

Should you accept, you'll get write access to the main repository. (and a fancy styled-components logo on your GitHub profile!) You'll be able to label and close issues, merge pull requests, create new branches, etc. We want you to help us out with those things, so don't be scared to do them! Make sure to read our contribution and community guidelines, which explains all of this in more detail. Of course, if you have any questions just let me know!

quantizor · 2018-06-04T05:53:59Z

Deployed in https://github.com/styled-components/styled-components/releases/tag/v3.3.2!

Allow Class attr in styled-component

80eebaf

valerybugakov changed the title ~~Allow Class attr in styled-component~~ Allow ES6 class attrs in styled-component May 22, 2018

Added changelog entry

ab84987

quantizor reviewed May 24, 2018

View reviewed changes

quantizor reviewed May 25, 2018

View reviewed changes

Check if React.Component is in prototype chain of attr

cd6df77

valerybugakov force-pushed the fix/allow-class-attrs branch from 66dafc2 to cd6df77 Compare May 25, 2018 08:22

quantizor reviewed May 29, 2018

View reviewed changes

quantizor previously approved these changes May 29, 2018

View reviewed changes

Inline inheritance check

0e18d00

valerybugakov dismissed quantizor’s stale review via 0e18d00 May 29, 2018 05:35

valerybugakov added 2 commits May 30, 2018 18:58

Deleted test-results.json

1313930

Merge branch 'master' into fix/allow-class-attrs

5295d7a

Bypass mysterious flow errors for now

b3bf258

ignore one more flow issue

e13bf33

disable danger for the time being

26f2535

since travis is not allowing encrypted variables to be available to forks, it makes running danger impossible for most PRs I'm going to look into alternate services making use of the new Github Apps API that don't have this restriction.

quantizor merged commit 24b03e7 into styled-components:master Jun 1, 2018

quantizor mentioned this pull request Jun 2, 2018

Styled-component breaks a library on production mode #1687

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow ES6 class attrs in styled-component #1751

Allow ES6 class attrs in styled-component #1751

valerybugakov commented May 22, 2018

quantizor May 24, 2018

valerybugakov May 24, 2018

quantizor May 25, 2018

valerybugakov May 25, 2018

quantizor May 25, 2018

valerybugakov May 25, 2018

valerybugakov commented May 25, 2018

quantizor May 29, 2018

valerybugakov May 29, 2018

quantizor left a comment

valerybugakov commented May 30, 2018

quantizor commented May 30, 2018

valerybugakov commented May 30, 2018

quantizor commented May 30, 2018

valerybugakov commented May 30, 2018

quantizor commented Jun 1, 2018

valerybugakov commented Jun 1, 2018

quantizor commented Jun 1, 2018

mxstbr commented Jun 1, 2018

quantizor commented Jun 4, 2018

Allow ES6 class attrs in styled-component #1751

Allow ES6 class attrs in styled-component #1751

Conversation

valerybugakov commented May 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valerybugakov commented May 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quantizor left a comment

Choose a reason for hiding this comment

valerybugakov commented May 30, 2018

quantizor commented May 30, 2018

valerybugakov commented May 30, 2018

quantizor commented May 30, 2018

valerybugakov commented May 30, 2018

quantizor commented Jun 1, 2018

valerybugakov commented Jun 1, 2018

quantizor commented Jun 1, 2018

mxstbr commented Jun 1, 2018

quantizor commented Jun 4, 2018