Skip to content
This repository was archived by the owner on Jun 10, 2020. It is now read-only.

Modernize react-infinite#235

Merged
garetht merged 38 commits intomasterfrom
upgrades
Aug 7, 2017
Merged

Modernize react-infinite#235
garetht merged 38 commits intomasterfrom
upgrades

Conversation

@garetht
Copy link
Copy Markdown
Contributor

@garetht garetht commented Aug 5, 2017

  • Moves from createClass to ES2015 class
  • Adds prettier and upgrades eslint
  • Upgrades jest to version 20 and updates to snapshot tests
  • Upgrades flow to version 0.52 and updates types
  • Changes travis settings and sets up a cache for node_modules
  • Permanently removes deprecated infiniteLoadBeginBottomOffset, deprecated since 0.6.0

Moving to Webpack 2 and ES2015 imports will likely be done in a future pull request.

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 3c366e9 on upgrades into ** on master**.

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 5c6ebab on upgrades into ** on master**.

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 6ee181e on upgrades into ** on master**.

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling b2cc7ac on upgrades into ** on master**.

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 22ae28f on upgrades into ** on master**.

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 3848461 on upgrades into ** on master**.

Copy link
Copy Markdown
Member

@chrisvoll chrisvoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a couple comments, but these changes look great! Nice to see cypress used in production too

Comment thread __tests__/infinite_test.js Outdated
}).toThrow();
}
// expect(rootNode.topSpacer._style._values.height).toEqual('400px');
// expect(rootNode.bottomSpacer._style._values.height).toEqual('1600px');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove these commented out tests if they're no longer relevant :)

Comment thread __tests__/infinite_test.js Outdated
</Infinite>
);

expect(rootNode).toMatchSnapshot();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not so sure that a snapshot does a better job of capturing this than explicitly looking for pointer-events being undefined. The test's description is much more specific than the test itself

Comment thread __tests__/infinite_test.js Outdated
waitsFor(function() {
return infiniteSpy.callCount > 0;
});
const infiniteSpy = jasmine.createSpy('infiniteSpy');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jasmine APIs are no longer officially supported within jest, so for future compatibility these should be replaced with jest.fn()

@@ -0,0 +1,22 @@
/* eslint-disable */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which eslint rules does this break? If it's just the cy variable, you could drop a .eslintrc config in the cypress directory with cy as a global

cy.get("#infinite-example-one").as('basic');

// Shows only
for (var i = 0; i < 8; i++) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let i

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 9549cd3 on upgrades into ** on master**.

@coveralls
Copy link
Copy Markdown

Coverage Status

Changes Unknown when pulling 1596a17 on upgrades into ** on master**.

@garetht garetht merged commit 7efd664 into master Aug 7, 2017
@garetht garetht deleted the upgrades branch August 7, 2017 14:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants