test(Examples): assert no console errors #645

levithomason · 2016-10-08T05:35:28Z

Fixes #604
Fixes #599

This PR adds a test that renders all doc site examples and asserts that no console errors were logged. I'll fix the errors as well before merging. I may also extend this to render all doc site routes instead, that way every page is covered and we know there are no console errors in the docs.

FAILED TESTS:
  examples
    ✖ render without errors
      PhantomJS 2.1.1 (Mac OS X 0.0.0)
      Example File: ./collections/Menu/Types/Attached.js
      Console Error: Warning: Failed prop type: `children` prop in `Dropdown` requires props: `text`.
          in Dropdown

      Example File: ./elements/Input/Variations/InputActionDropdown.js
      Console Error: Warning: Failed prop type: `options` prop in `Dropdown` requires props: `selection`.
          in Dropdown

      Example File: ./views/Card/Content/ContentBlock.js
      Console Error: Warning: Failed prop type: Prop `date` in `FeedContent` conflicts with props: `children`. They cannot be defined together, choose one or the other.
          in FeedContent

      : expected [ Array(3) ] to be empty

      Stack:
      test/specs/docs/examples-test.js:27:0

jeffcarbs · 2016-10-08T14:56:02Z

This is great!

While writing examples for List shorthand I discovered a bug in how the shorthand was being rendered. Got me thinking - could we automate tests for full markup and shorthand to ensure they produce the same html? This would expose errors in our use of the shorthand or errors within the components themselves - both things we should definitely fix! For example:

exampleContext.keys().forEach(path) => {
  if (!path.match(/Shorthand$/)) return

  // Makes use of the naming convention of examples
  // Full example: ListExampleBasic
  // Shorthand example: ListExampleBasicShorthand
  const fullExamplePath = path.replace(/Shorthand$/, '')

  const FullExample = createElement(exampleContext(fullExamplePath).default)
  const ShorthandExample = createElement(exampleContext(path).default)

  render(ShorthandExample).should.equal(render(FullExample))
});

levithomason · 2016-10-08T16:39:25Z

That sounds pretty sane. I think we'd need to call html() on the wrapper to compare the html strings, but the point still stands.

I also added Foo.create() common tests in #644. The Label create method returned undefined since it was passing Label inside of its own class. The common tests implementsCreateMethod just asserts it exists and creates elements for every supported shorthand value.

codecov-io · 2016-10-08T17:56:53Z

Current coverage is 96.11% (diff: 100%)

No coverage report found for master at c7f5dc8.

Powered by Codecov. Last update c7f5dc8...8a047a3

levithomason · 2016-10-08T17:59:04Z

I updated the test to mount() each example, which revealed far more errors. I've fixed them all except for those with key errors. We need to solve how factories will generate keys, or require the user to pass a key, in order to solve this.

Will pick this up later.

FAILED TESTS:
  examples
    ✖ renders without errors: ./collections/Table/States/TableWarningShorthand.js
      PhantomJS 2.1.1 (Mac OS X 0.0.0)
      Console error: Warning: Each child in an array or iterator should have a unique "key" prop. Check the render method of `Table`. See https://fb.me/react-warning-keys for more information.
          in TableRow (created by Table)
          in Table (created by Unknown)
          in Unknown (created by Unknown)
          in Unknown,Warning: Each child in an array or iterator should have a unique "key" prop. Check the render method of `TableRow`. See https://fb.me/react-warning-keys for more information.
          in TableCell (created by TableRow)
          in TableRow (created by Table)
          in thead (created by TableHeader)
          in TableHeader (created by Table)
          in table (created by Table)
          in Table (created by Unknown)
          in Unknown (created by Unknown)
          in Unknown: expected error to not have been called

      Stack:
      test/specs/docs/examples-test.js:21:0

    ✖ renders without errors: ./elements/List/Types/ListExampleBasicShorthand.js
      PhantomJS 2.1.1 (Mac OS X 0.0.0)
      Console error: Warning: Each child in an array or iterator should have a unique "key" prop. Check the render method of `List`. See https://fb.me/react-warning-keys for more information.
          in ListItem (created by List)
          in List (created by Unknown)
          in Unknown (created by Unknown)
          in Unknown: expected error to not have been called

      Stack:
      test/specs/docs/examples-test.js:21:0

levithomason · 2016-10-10T06:09:23Z

I've pushed some work in progress for creating key props for children. I think we should consider a createCollection method. This way, we know when to create keys.

The create method has no way of telling if it is being called due to mapping over a shorthand collection. Therefore, it has no way of knowing whether or not it should generate a key if none were provided.

If we allow a createCollection method, then we can reliably attempt to generate keys and also warn users in development mode if we so choose. Currently, I'm experimenting with generating a hash code key based on a shallow stringified final props object. So it's as unique as the full shallow prop signature. This is only if there were no key nor childKey provided. If we only do this when creating a collection (ie createCollection) then I think this could be the way to go.

levithomason · 2016-10-27T21:04:27Z

I've split createShorthandItem and createShorthandCollection. The latter handles retrieving child keys or generating them.

I just need to finish updating the TableRow.create to utilize the TableCell factory. It seems collection factories should be responsible for passing the item factory that handles their array values. Or, use it in the map value to props method.

jeffcarbs · 2016-10-27T22:10:13Z

src/collections/Table/Table.js

@@ -84,7 +84,7 @@ function Table(props) {
    <ElementType {...rest} className={classes}>
      {headerRow && <TableHeader>{TableRow.create(headerRow, { cellAs: 'th' })}</TableHeader>}
      <TableBody>
-        {renderBodyRow && _.map(tableData, (data, index) => TableRow.create(renderBodyRow(data, index)))}
+        {renderBodyRow && TableRow.create(_.map(tableData, (data, index) => renderBodyRow(data, index)))}


I think this change is incorrect. This should map over each item in the tableData array and create a TableRow from each one. This change would map over each item in the tableData array and create one giant TableRow.

Thanks, I've got some updating to do here. Had to switch to something else. I believe the TableRow.create should be mapping the cells over TableCell.create to achieve what I'm after. We'll see, I need to play with it to grok it.

levithomason · 2016-11-22T21:36:54Z

docs/app/Examples/views/Feed/Content/FeedExampleDate.js

@@ -1,47 +0,0 @@
-import React from 'react'


Removed cruft example file, it was broken down into separate examples but never deleted.

levithomason · 2016-11-22T21:37:08Z

docs/app/Examples/views/Feed/Content/FeedExampleDateShorthand.js

@@ -1,24 +0,0 @@
-import React from 'react'


Same here, broken down, never deleted.

levithomason · 2016-11-26T18:47:38Z

Interestingly in React v15.4 there are no longer warnings for missing keys nor duplicate keys. It still appears missing or duplicate keys results in overridden child nodes when rendered to the DOM.

I believe I solved all the remaining issues. I'll come back and clean up and confirm this when I get time.

levithomason · 2016-11-27T18:58:30Z

Haha, there are no warnings because the docs started using React min via CDN...

levithomason

Woop! I believe this ready to go! I've self-reviewed and add some inline notes. Would be great to get some eyes on this from one of @jcarbo, @davezuko, or @layershifter.

Thanks much!

levithomason · 2016-12-06T07:19:31Z

docs/app/Examples/collections/Menu/Types/MenuExampleAttached.js

@@ -10,7 +10,7 @@ const MenuExampleAttached = () => {
        <Dropdown as={Menu.Item} icon='wrench' simple>
          <Dropdown.Menu>
            <Dropdown.Item>
-              <Icon name='dropdown icon' />
+              <Icon name='dropdown' />


You'll see several fixes to the docs like this one.

levithomason · 2016-12-06T07:22:43Z

docs/app/Examples/elements/List/Content/ListExampleDescription.js

@@ -4,14 +4,14 @@ import { List } from 'semantic-ui-react'
 const ListExampleDescription = () => (
  <List>
    <List.Item>
-      <List.Icon name='map marker' />
+      <List.Icon name='marker' />


This was wrong in core docs as well. There is on a map or marker icon, not a map marker. The prop type checker is sooo helpful for these :D

levithomason · 2016-12-06T07:24:29Z

docs/app/utils.js

@@ -18,7 +18,7 @@ export const parentComponents = _.flow(
 /**
 * Get the Webpack Context for all doc site examples.
 */
-export const exampleContext = require.context('docs/app/Examples/', true, /\.js$/)
+export const exampleContext = require.context('docs/app/Examples/', true, /(\w+Example\w+|index)\.js$/)


This now excludes non-renderable other files, like common.js. It is used in a few places, but only for rendering the example view components themselves.

P.S. The common.js files actually need to go as we cannot truly import any files into the examples since we're in a browser. That is for another PR.

levithomason · 2016-12-06T07:25:11Z

src/collections/Breadcrumb/BreadcrumbSection.js

@@ -93,4 +93,4 @@ export default class BreadcrumbSection extends Component {
  }
 }

-BreadcrumbSection.create = createShorthandFactory(BreadcrumbSection, content => ({ content, link: true }))
+BreadcrumbSection.create = createShorthandFactory(BreadcrumbSection, content => ({ content, link: true }), true)


This is how we now flag a factory to generateKey. It will generate a key as a hash of shallow props.

levithomason · 2016-12-06T07:51:48Z

src/lib/customPropTypes.js

@@ -251,6 +251,7 @@ export const contentShorthand = (...args) => every([
 export const itemShorthand = (...args) => every([
  disallow(['children']),
  PropTypes.oneOfType([
+    PropTypes.array,


Fixes warnings with tableFooter. This prop is shorthand for a Row but the array literal case is passed to the row cells via mapValueToProps. Since the shorthand item factory supports arrays, the shorthand item type should as well.

levithomason · 2016-12-06T07:54:11Z

src/lib/factories.js

  }
+  return hash
+}


When we generate keys we make a prop string, then we make a hash of that. The only added benefit of the hash is having very short keys. The downside is more processing at render time. We could just leave the shallow stringified props as the key. DOM weight is not an issue here as the key is in memory only.

I'm on the brink of dropping it already, LMK your thoughts.

levithomason · 2016-12-06T07:56:31Z

src/lib/getUnhandledProps.js

@@ -37,6 +37,7 @@ const getUnhandledProps = (Component, props) => {
  // Return _unhandled_ props
  // ----------------------------------------
  return Object.keys(props).reduce((acc, prop) => {
+    if (prop === 'childKey') return acc


Always "handle" the childKey prop so we don't apply it in factories.

levithomason · 2016-12-06T07:58:07Z

test/specs/collections/Table/Table-test.js

+    const renderBodyRow = ({ name, status, notes }) => [
+      name || { key: 0 },
+      status || { key: 1 },
+      notes || { key: 2 },


Since these are cells and in the event that there are multiple missing cells in the tableData (which there are) then the cells will all have identical props objects and therefore identical generated keys.

I've instead defaulted to objects with a key defined.

levithomason · 2016-12-06T07:58:33Z

test/specs/docs/examples-test.js

+    if (/index\.js$/.test(path)) return
+    const filename = path.replace(/^.*\/(\w+\.js)$/, '$1')
+
+    it(`${filename} renders without console messages`, () => {


…handProp

levithomason · 2016-12-16T01:47:34Z

🎉 🎉 🎉

* pr/2: docs(Sidebar): fix filenames fixes Semantic-Org#1048 docs(changelog): update changelog [ci skip] 0.62.3 feat(Checkbox): Indeterminate state (Semantic-Org#1043) Feat(typings): Added typings for Radio, Confirm, Select and Textarea (Semantic-Org#1047) chore(package): update babel-core to version 6.21.0 (Semantic-Org#1045) fix(dropdown): clear value when dropdown is blurred (Semantic-Org#1046) fix(typings): fix typings spelling on `Accordion` (Semantic-Org#1044) fix(docs): Fix check icon (Semantic-Org#1042) docs(changelog): update changelog [ci skip] 0.62.2 fix(Header): Remove duplicate class (Semantic-Org#1040) test(Examples): assert no console errors (Semantic-Org#645) feat(Accordion): ability to open multiple items (Semantic-Org#988) feat(docs): Active className on Sidebar item (Semantic-Org#1031) chore(package): update node-sass to version 4.0.0 (Semantic-Org#1015) chore(package): update debug to version 2.4.1 (Semantic-Org#1028)

levithomason mentioned this pull request Oct 8, 2016

docs(Example): Fix warnings #599

Closed

17 tasks

This was referenced Oct 14, 2016

Dropdown trigger generate warning #676

Closed

feat(Dropdown): Refactor dropdown #672

Closed

levithomason force-pushed the chore/test-example-errors branch 2 times, most recently from 5282261 to a68c279 Compare October 27, 2016 21:01

jeffcarbs reviewed Oct 27, 2016

View reviewed changes

levithomason mentioned this pull request Oct 29, 2016

RFC: Open questions around "Collection Shorthand" #604

Closed

levithomason force-pushed the chore/test-example-errors branch from a68c279 to 7431f0c Compare November 22, 2016 21:33

levithomason commented Nov 26, 2016

View reviewed changes

levithomason force-pushed the chore/test-example-errors branch from 2055ddd to e30f79b Compare November 30, 2016 20:39

levithomason mentioned this pull request Dec 5, 2016

Label click for input element (accessibility) #980

Closed

levithomason force-pushed the chore/test-example-errors branch from e30f79b to ab13bf0 Compare December 6, 2016 07:18

levithomason commented Dec 6, 2016

View reviewed changes

levithomason mentioned this pull request Dec 14, 2016

fix(Table): Keys warning when using renderBodyRow #1030

Closed

levithomason added 8 commits December 15, 2016 16:43

test(Examples): assert no console errors

c0ba04f

docs(Feed): remove unused examples

ed0681b

docs(Card): fix Feed.Content date usage

6abe1a3

docs(Menu): fix Input icon shorthand

036289a

test(Examples): assert no console errors on mount

b2c078c

wip shorthand collection keys

2685c55

fix(Header): remove unused import

3f9ebe2

fix(Image-test): update implementsShorthandProp name

b17d67a

levithomason added 21 commits December 15, 2016 16:43

fix(Table): restore tableData mapping logic

0cd3b32

refactor(Table): fix doc block

88ef232

refactor(factories): use arrow func over _.partial

9d4fcde

refactor(factories): merge item/collection factories

f911a33

fix(customPropTypes): allow array in itemShorthand

571099c

refactor(factories): cleanup cruft

c7eb30a

fix(getUnhandledProps): always handle childKey

25a6d46

refactor(commonTests): implementsShorthandItemProp -> implementsShort…

5f2c6c1

…handProp

test(examples): assert no console messages

4dd1d98

fix(docs|src): fix all example errors

c5165c4

fix(Introduction): fix icon name

1e2e861

fix(Menu): add MenuItem.create() and generateKey

6ff93b8

fix(MessageList): add MessageItem.create() and generateKey

95caa94

fix(Message): add Header.create() and List.create()

1827b8a

refactor(factories): one line if

9128bed

fix(Menu): removed unused import

1e6c783

chore(karma): fix html-beautify module error

d3b095a

refactor(factories): do not hash prop keys

6c499ac

test(Menu): fix menu test

562e406

test(Message): fix message test

74b9417

chore(webpack): remove unused html-beautify external

8a047a3

levithomason force-pushed the chore/test-example-errors branch from c6736fb to 8a047a3 Compare December 16, 2016 01:41

levithomason merged commit c7fc9d4 into master Dec 16, 2016

levithomason deleted the chore/test-example-errors branch December 16, 2016 01:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(Examples): assert no console errors #645

test(Examples): assert no console errors #645

levithomason commented Oct 8, 2016 •

edited

Loading

jeffcarbs commented Oct 8, 2016

levithomason commented Oct 8, 2016

codecov-io commented Oct 8, 2016 •

edited

Loading

levithomason commented Oct 8, 2016

levithomason commented Oct 10, 2016 •

edited

Loading

levithomason commented Oct 27, 2016

jeffcarbs Oct 27, 2016

levithomason Oct 27, 2016

levithomason Nov 22, 2016

levithomason Nov 22, 2016

levithomason commented Nov 26, 2016

levithomason commented Nov 27, 2016

levithomason left a comment

levithomason Dec 6, 2016

levithomason Dec 6, 2016

levithomason Dec 6, 2016

levithomason Dec 6, 2016

levithomason Dec 6, 2016

levithomason Dec 6, 2016

levithomason Dec 6, 2016

levithomason Dec 6, 2016

levithomason Dec 6, 2016

levithomason commented Dec 16, 2016

test(Examples): assert no console errors #645

test(Examples): assert no console errors #645

Conversation

levithomason commented Oct 8, 2016 • edited Loading

jeffcarbs commented Oct 8, 2016

levithomason commented Oct 8, 2016

codecov-io commented Oct 8, 2016 • edited Loading

Current coverage is 96.11% (diff: 100%)

levithomason commented Oct 8, 2016

levithomason commented Oct 10, 2016 • edited Loading

levithomason commented Oct 27, 2016

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

levithomason commented Nov 26, 2016

levithomason commented Nov 27, 2016

levithomason left a 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

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

levithomason commented Dec 16, 2016

levithomason commented Oct 8, 2016 •

edited

Loading

codecov-io commented Oct 8, 2016 •

edited

Loading

levithomason commented Oct 10, 2016 •

edited

Loading