Skip to content

Comments

fix: eslint warnings#1877

Open
babygoat wants to merge 1 commit intotwreporter:masterfrom
babygoat:eslint-fix
Open

fix: eslint warnings#1877
babygoat wants to merge 1 commit intotwreporter:masterfrom
babygoat:eslint-fix

Conversation

@babygoat
Copy link
Collaborator

@babygoat babygoat commented Jun 3, 2021

This patch fixes several eslint warnings and adds the generated
sw.js to ignore error.

This patch fixes several eslint warnings and adds the generated
sw.js to ignore error.
@babygoat babygoat requested a review from taylrj June 3, 2021 07:25
@babygoat babygoat self-assigned this Jun 3, 2021
@babygoat
Copy link
Collaborator Author

babygoat commented Jun 3, 2021

@taylrj Only react/no-find-dom-node warnings are not resolved. I'm curious why u prefer to enforce the rule explicitly. Do u have any plans to migrate the API to react refs?

@taylrj
Copy link
Contributor

taylrj commented Jun 3, 2021

LGTM.

Yes, there's a plan to migrate the API since findDOMNode is going to be deprecated.
The alternative API usage needs some investigation, we don't have to include the change in this PR.

@babygoat
Copy link
Collaborator Author

babygoat commented Jun 3, 2021

The reason why I do the eslint fix is that I would like to enable the linter on CI but the warnings will break it. AFAIK, we need to put the refs on the child elements and use it for offset and height calculation, right?

@taylrj
Copy link
Contributor

taylrj commented Jun 7, 2021

AFAIK, we need to put the refs on the child elements and use it for offset and height calculation, right?

In theory, yes.

But I haven't figure out the way doing so and keep current behavior at the same time.
It might need more time to investigate how to refactor away use of findDOMNode.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants