Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Test against React 17 #15732

Closed
ecraig12345 opened this issue Oct 27, 2020 · 10 comments
Closed

Test against React 17 #15732

ecraig12345 opened this issue Oct 27, 2020 · 10 comments
Assignees
@ecraig12345
Labels
Area: Build System React 17
Projects
V-Build - @microsoft/fluentui-react-b...

Comments

@ecraig12345
Copy link
Member

Our minimum supported React version for @fluentui/react 8 will remain at 16.8, but we should also investigate adding || ^17.0.0 to our peer dependency range.
@ecraig12345 ecraig12345 created this issue from a note in Version 8 (To do: Build, packaging, website) Oct 27, 2020
@ecraig12345 ecraig12345 added this to the v8: Pre-release milestone Oct 27, 2020
@ecraig12345 ecraig12345 changed the title Investigate React 17 support (as an option) React 17 support (as an option) Oct 27, 2020
@ecraig12345
Copy link
Member Author

@xugao started testing this out in #15796 but we ran into some test issues due to lack of React 17 support in Enzyme (enzymejs/enzyme#2429). So this is on hold (hopefully not for too long) until we can actually run our tests. There were also some concerning initial perf results.

@xugao
Copy link
Contributor

xugao commented Nov 3, 2020

Quick update:
For unit test, I tried updating the code to use @wojtekmaj/enzyme-adapter-react-17. some tests need to wrap act when unmount to pass; some tests failed which appeared to be focus is not working on native element, which i have not root caused yet. See commit.

@trueadm
Copy link

trueadm commented Nov 3, 2020

some tests failed which appeared to be focus is not working on native element, which i have not root caused yet.

You'll likely need to use focusin rather than focus. You'll also need to use focusout rather than blur. This is because React now uses these native events under the hood.

@ecraig12345 ecraig12345 mentioned this issue Nov 9, 2020
Closed
@ecraig12345 ecraig12345 mentioned this issue Jan 18, 2021
Closed
@ecraig12345 ecraig12345 mentioned this issue Jan 25, 2021
Closed
@ecraig12345 ecraig12345 mentioned this issue Feb 9, 2021
Closed
2 tasks
@ecraig12345
Copy link
Member Author

some tests failed which appeared to be focus is not working on native element, which i have not root caused yet.

You'll likely need to use focusin rather than focus. You'll also need to use focusout rather than blur. This is because React now uses these native events under the hood.

@trueadm Do you mean in tests, or in real code? I think the tests in question are calling focus() on a native element, not listening for events.

@ecraig12345 ecraig12345 moved this from To do: Build, packaging, website to In progress in Version 8 Feb 18, 2021
This was referenced Feb 18, 2021
Merged
Closed
@MwSpaceLLC
Copy link

😍 Hi guys,
I just wanted to know when it will be available for our dash project with react "17.0.1".
I ask this question so I can move the team to other parts of development

@ecraig12345
Copy link
Member Author

@MwSpaceLLC It's available in the version 8 beta currently.

@ecraig12345 ecraig12345 moved this from In progress to To do: Build, packaging, website in Version 8 Feb 24, 2021
@ecraig12345 ecraig12345 changed the title React 17 support (as an option) React 17 support option + test against 17 Feb 24, 2021
@ecraig12345 ecraig12345 changed the title React 17 support option + test against 17 React 17 support as an option + test against 17 Feb 24, 2021
@ecraig12345 ecraig12345 changed the title React 17 support as an option + test against 17 React 17 support (as an option) + test against 17 Feb 24, 2021
@MwSpaceLLC
Copy link

Tnks @ecraig12345! So u have any date for use it on production? Or we can try to use it in production? Sorry my question 😢

@ecraig12345 ecraig12345 assigned ecraig12345 and unassigned xugao Feb 25, 2021
@ecraig12345
Copy link
Member Author

@MwSpaceLLC Version 8 (non-beta) was released today. If you find any issues with it using React 17, please file an issue and we'll fix it.

@AlexFsmn
Copy link

AlexFsmn commented Apr 23, 2021 •
edited

@ecraig12345
I've been waiting 2 months for this to be fixed:
#17313

@ecraig12345 ecraig12345 changed the title React 17 support (as an option) + test against 17 Test against React 17 May 25, 2021
@ecraig12345 ecraig12345 removed this from To do: Build, packaging, website in Version 8 May 25, 2021
@ecraig12345 ecraig12345 added this to Backlog in V-Build - @microsoft/fluentui-react-build via automation May 25, 2021
@ecraig12345 ecraig12345 removed this from the v8: Post-release milestone May 25, 2021
@ecraig12345 ecraig12345 mentioned this issue Oct 15, 2021
Closed
@ecraig12345
Copy link
Member Author

Closing in favor of #20145.

V-Build - @microsoft/fluentui-react-build automation moved this from Backlog to Done Dec 8, 2021
@ecraig12345 ecraig12345 mentioned this issue Dec 14, 2021
Merged
2 tasks
@microsoft microsoft locked as resolved and limited conversation to collaborators Jan 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@ecraig12345 ecraig12345

Labels
Area: Build System React 17
Projects
No open projects
V-Build - @microsoft/fluentui-react-b...
  
Done
Milestone
No milestone
Development

No branches or pull requests
5 participants
@xugao @trueadm @ecraig12345 @AlexFsmn @MwSpaceLLC