Upgrade to React 18

[ExtraBB]

Upgrading to React 18 (#115)

Change Description:

- Upgrade `react` and `react-dom` to version 18
- Remove `react-hot-loader` because this is not usable with React 18 anymore. React 18 provides hot reloading out-of-the-box

1. Do these changes impact any npm scripts commands (in package.json)? (e.g., running 'npm run start') No

2. Do these changes impact VS Code debugging options (launch.json)? No

3. Do these changes impact template output? (e.g., add/remove file, update file location, update file contents) Yes

4. Do these changes impact documentation? (e.g., a tutorial on https://docs.microsoft.com/en-us/office/dev/add-ins/overview/office-add-ins) No

If you answered yes to any of these please do the following: @Rick-Kirkham

Validation/testing performed:

• Tested editing code to see if Hot-reload is still working. (It is)

[ExtraBB]

@microsoft-github-policy-service agree

[millerds]

/azurepipelines run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[millerds]

Looks like you need to correct some linter errors.

[ExtraBB]

/azurepipelines run

[azure-pipelines[bot]]

Commenter does not have sufficient privileges for PR 116 in repo OfficeDev/Office-Addin-TaskPane-React

[millerds]

/azurepipelines run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[ExtraBB]

I see that I am getting the same issue as #90 . Let me see if I can reproduce it this time

[ExtraBB]

Can anyone tell me why the tests are failing? I can't access the Azure Pipelines. Also, should we make the pipeline logs public somehow so this isn't a problem in the future?

[millerds]

/azurepipelines run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[millerds]

The security policy we have requires an internal person to trigger the pipeline manually for external submissions. Since you submitted the changes to the PR the previous results were hidden waiting for the new results that I had to trigger (sorry you had to wait).

It looks like the edge legacy (a.k.a. edgeHtml view) test passed this time. The IE test failed, but there was another problem recently reported (just a week or two ago) that suggested it won't pass on IE. I think this should be OK.

[millerds]

Hmm . . . actually looks like the master branch was able to pass the automation even with this other issue reported. Not sure what is blocking this IE test from passing.

[millerds]

/azurepipelines run

[millerds]

@ExtraBB I had a machine setup to directly try an older build of office so I took a look at your PR there. Looks like these changes introduce a need for a polyfill in order for IE11 webivew to work. The error is "'Symbol' is undefined". I'm not sure which part of the change caused this and I haven't looked into specifically what to include to provide the polyfill. That would need to be fixed before checking in the change.

[ExtraBB]

Thanks for checking out the error! It seems that some things are not polyfilled correctly. React 18 requires the following polyfills: https://legacy.reactjs.org/docs/javascript-environment-requirements.html.

The problem must lie in the webpack config somewhere. Things I changed in my last commit:

• I noticed that the polyfill chunk does not have the same name that is referenced in the HtmlWebpackPlugin section for the taskpane.html so I made the names match.
• Made the HtmlWebpackPlugin section load the chunks in manual order, ensuring the polyfill chunk is loaded first.

Let's try this simple fix and see if it fixes the pipelines. If not, I think the solutions in this thread might be interesting as a next step: https://github.com/zloirock/core-js/issues/514.

[millerds]

/azurepipelines run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[ExtraBB]

Since the last pipeline failed, I dove a bit into the issue mentioned above (https://github.com/zloirock/core-js/issues/514) and it seems that for these people the same problem is caused by either:

• Importing core-js twice
• Accidentally transpiling core-js with babel

Looking at our webpack.config.js, I don't see how babel would be able to transpile core-js unless I'm missing something. There is a double usage of core-js so I removed that in my latest commit. Let's hope this fixes it.

To be honest, if this doesn't work I am out of ideas. Any ideas from your end @millerds ? Another question, do you know for how long IE will have to keep being supported? I noticed that on the web, IE can't even open excel workbooks on sharepoint.

[millerds]

I think the main issue is that the misspelling of "polyfill" in both the main webpack config and the test case. Once that is corrected (I'm not sure how that managed to get in or stay there) a new compatibility problem is exposed in one of the src files. I'm looking at putting together a general polyfill correction PR that I can verify directly on my machine. Once that is corrected and merged into this PR I hope it will be good.

[millerds]

@ExtraBB I made a general polyfill fix (that includes an update to a component to be compatible with IE11), but I think we're going to run into a new problem. @fluentui/react is used in this template and they made some changes to their package starting at version 8.69 that are not compatible with IE11 (they aren't transpiling from the use of '=>' in one of their modules), but that version of @fluentui/react also only indicates compatibility with react version less than 18. So in order to use react 18+ and newer versions of @fluentui/react we need to figure out how to transpile the imported module.

[ExtraBB]

@millerds We could transpile the node modules as well, with the exception of core-js. Like in this answer from the issue I shared earlier: https://github.com/zloirock/core-js/issues/514#issuecomment-523524472. Would that be an acceptable solution or too big of a change? Alternatively we could transpile just the module you mentioned, which module is it?

[millerds]

I had seen similar solutions on the internet myself. I think would be reasonable to do that for the single module (@fluentui/react-portal-compat-context) or the group of @fluentui/* modules.

[ExtraBB]

Updated @fluentui/react to latest and made sure it's transpiled. Could you run the pipelines?

[millerds]

/azurepipelines run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[millerds]

That didn't work. Output sill used '=>'.

One way to check this is to run 'npm run build' and the search the files in the dist folder for '=>'. As long as it's there (not in strings or comment) then it will fail in IE. If you want to do a full test you can install an older version of office using the instructions at https://learn.microsoft.com/en-us/office/dev/add-ins/testing/ie-11-testing#install-a-version-of-office-that-uses-internet-explorer.

Since it's an imported module, my guess is that the files aren't typescript, but rather js. You'll probably need to make a new loader rule for .js and make it only include fluentui folder in node_modules.

[Rick-Kirkham]

Closing this PR because it is now redundant, given https://github.com/OfficeDev/Office-Addin-TaskPane-React/pull/128