Metro: Add 'desktop' platform to metro

[vkjr]

Do you want to request a feature or report a bug?

I want to request a feature.
I participate in development of react-native for desktop - https://github.com/status-im/react-native-desktop.
In that project, we are adding cross-platform desktop support to react-native. We use *.desktop.js files for desktop-specific implementation.
In order for that to work we are patching src/defaults.js file in metro to contain string:
exports.platforms = ['ios', 'android', 'windows', 'web', 'desktop'];

Can desktop platform be added to metro project officially?
Thanks.

What is the current behavior?

Currently when we run unpatched metro in react-native-desktop code following errors arise:

error: bundling failed: ambiguous resolution: module `...../RNDesktopTest/node_modules/react-native/Libraries/react-native/react-native-implementation.js` tries to require `Button`, but there are several files providing this module. You can delete or fix them:

  * `...../RNDesktopTest/node_modules/react-native/Libraries/Components/Button.desktop.js`
  * `...../RNDesktopTest/node_modules/react-native/Libraries/Components/Button.js`

What is the expected behavior?

Expected behavior - no errors in files resolution.

Please provide your exact Metro configuration and mention your Metro, node, yarn/npm version and operating system.
OS: MacOS, Windows, Ubuntu
npm: 5.5.1
metro: 0.24.7
node: 8.9.1

[rafeca]

Hey @vkjr ! I'm not very familiar with the architecture of react-native-desktop, but I guess that it still uses the rn-cli.conf.js configuration files. If so, have you tried adding a getPlatforms() param there returning [desktop]?

[madhavarshney]

@rafeca - Yes, that works, but we thought it might be better to add desktop platform support directly to metro as there are two other externally developed platforms in the defaults, web and windows. But at the same time, there are many other platforms that can be added, like macos, dom, and vr, so it might be a better idea to keep metro's defaults clean and to use rn-cli.config.js instead.

---

Currently react-native-desktop is a full-blown fork of react-native with the desktop platform added using Qt. I'm trying to convert it to a react-native and rnpm extension, but I'm having trouble with metro. _Should I open a new issue?_ Here's the error:

error: bundling failed: Error: Unable to resolve module `AccessibilityInfo` from `D:\.....\RNDesktopTest\node_modules\react-native\Libraries\react-native\react-native-implementation.js`: Module does not exist in the module map or in these directories:
  D:\.....\RNDesktopTest\node_modules

This might be related to https://github.com/facebook/react-native/issues/4968
To resolve try the following:
  1. Clear watchman watches: `watchman watch-del-all`.
  2. Delete the `node_modules` folder: `rm -rf node_modules && npm install`.
  3. Reset Metro Bundler cache: `rm -rf $TMPDIR/react-*` or `npm start -- --reset-cache`.  4. Remove haste cache: `rm -rf $TMPDIR/haste-map-react-native-packager-*`.

It seems that the desktop modules are not being loaded into metro's module map. Do you know why metro is giving that error? Here is the rn-cli.config.js of a test project:

module.exports = {
  getPlatforms: () => ['ios', 'android', 'desktop'],
  getProvidesModuleNodeModules() {
    return ["react-native", "react-native-desktop"];
  },
};

Notes: react-native-desktop is yarn-linked in the test project. WIP refactor code at madhavarshney/react-native-desktop#RefactorRepo

[vincentriemer]

My two cents: I don't think it should be metro's responsibility to manage a list of "supported platforms" and instead I think React Native's packager should allow for 3rd party platforms to provide the required metro configuration additions (perhaps through the rnpm infrastructure).

In react-native-dom I have this super hacky ast transformation that I use in project generation to attempt to inject the necessary config changes and I'd jump at any opportunity to get rid of it :)

[madhavarshney]

@vincentriemer - Yes, I agree. Having the windows platform preconfigured in metro breaks windows support after creating a fresh project, adding windows support, and then adding dom support as react-native-windows relies on having windows and react-native-windows in the defaults of metro for getPlatforms and getProvidesModuleNodeModules, respectively, but react-native-dom overrides the defaults with only ['android', 'ios', 'dom'] in rn-cli.config.js. Perhaps the rnpm-plugin detection logic in react-native can be used to detect platforms as well.

[madhavarshney]

Currently react-native-desktop is a full-blown fork of react-native with the desktop platform added using Qt. I'm trying to convert it to a react-native and rnpm extension, but I'm having trouble with metro. Should I open a new issue? Here's the error:

Nevermind, I figured it out. npm/yarn links did cause an issue, but not that error.

[vkjr]

@rafeca, yes, we can go with an rn-cli.conf.js. It works but I thought that having a desktop platform in the initial list of supported ones would be cleaner.
Is there any possible drawbacks if it is added?