search

microsoft / react-native-windows

A framework for building native Windows apps with React.
https://microsoft.github.io/react-native-windows/
Other
16.35k stars 1.14k forks source link

Getting Started guide doesn't mention overwriting metro.config.js #4504

Closed kmelmon closed 4 years ago

kmelmon commented 4 years ago

With the current steps in the Getting Started guide, when you run "npx react-native windows" you'll get this prompt during the install process:

metro.config.js has changed in the new version. Do you want to keep your metro.config.js or replace it with the latest version? If you ever made any changes to this file, you'll probably want to keep it. You can see the new version here: D:\startme\node_modules\react-native-windows\local-cli\generator-windows\templates\metro.config.js Do you want to replace metro.config.js? Answer y to replace, n to keep your version:

The correct answer is universally yes - if you say no, you end up with the stock metro.config.js, which is guaranteed to not work with Windows.

This is causing confusion for customers. We should add language to the Getting Started guide to help avoid this. I can think of 2 possible solutions: 1) Tell them to supply the --overwrite flag. This will overwrite all files that are being installed that have the same name as anything already existing. This should be safe for a brand new project as there aren't yet any files other than the ones we're installing. 2) Tell people to say yes to the prompt to overwrite metro.config.js

I lean towards option 1.

chrisglein commented 4 years ago

Could we edit the CLI's output here to get a less scary / more contextual error message? Then we're not relying on the documentation. We probably want the documentation to mention it regardless, so that's a good first step.

chrisglein commented 4 years ago

@kikisaints the PR that was just taken adds an overwrite argument (are we comfortable with the other side effects that could have in the future for non-metro files? what other options have we considered), but doesn't add commentary as to why that's needed or notify people that we're going to overwrite any metro config.

harinikmsft commented 4 years ago

Given that this is a point in time issue that will go away once @kmelmon's changes to metro come through and per @kmelmon's original comment, the only way to get npx react-native-windows-init to work is by overwriting, I feel comfortable with the default on getting started specifying the --overwrite flag.

I do agree that we should provide some context on why the overwrite is needed and the fact that this will go away in the near term once the metro issues are fixed.

@kikisaints - perhaps, add something like this after the npx command in the getting started page:

The --overwrite flag is a temporary measure that ensures the correct files are copied to metro.config.js for the metro bundler to work with Windows. If you are starting a new app, this should have no impact. If you are adding Windows to your existing app and you have modified the metro.config.js file, please back up your changes, run the command and copy over to take effect.

@kmelmon - can you confirm if what I have quoted above is correct?

kmelmon commented 4 years ago

The fix was actually done by acoates (we took his alternative solution).
https://github.com/microsoft/react-native-windows/pull/4576 This solution currently still requires a custom metro.config.js so for now we need to overwrite it.

There is a proposal to implement this solution directly in the react-native CLI: react-native-community/cli#1115 This change would eventually eliminate the need for a custom metro.config, but it will take a while for this change to finally make it to us, and then for us to change our CLI to use it.