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.

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

Proposal: Allow typescript to type check modules using react-native's module resolution #21926

Closed
acoates-ms opened this issue Feb 14, 2018 · 15 comments · Fixed by #48189
Closed
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@acoates-ms
Copy link

Proposal is to add a new command line parameter to tsc which provides a list of resolutionPlatforms. This is just an array of strings, that will be used during that invocation of tsc to do module resolution.
You can then during your build invoke tsc once for each platform to do a complete static type check of the files that will actually get included by the metro-bundler.

This small change will allow developers to take existing typescript react-native projects which are not being properly typechecked due to metro-bundler's platform specific file resolution.

We have run this on our repo and found various bugs with the additional type checking.
This fixes the issue described in
#17681 which was closed due to no actionable solution.

The code for such a new parameter is relatively easy to add to typescript:
Here is a link to a branch based on 2.7.1 with the new parameter implemented.
acoates-ms@f023961

It also solves #8328 which has an incorrect solution of using the paths option, which doesn't work in a bunch of cases, such as relative path imports.
and was incorrectly marked as a dup of #420 , which isn't the same as that forces all the platforms to expose the same interface, which isn't what the bundler does.

Example build script to check multiple platforms:

/*
  By default tsc doesn't do platform resolution the same way as metro-bundler.
  This means that tsc wasn't checking the code exactly as its going to be in the bundle.

  This script manually runs tsc against the various platforms index entry points,
  modifiying the resolution platforms parameter according to the platform resolution
  rules of that platform.

  We can't just have the tsconfig do the right thing, as it can only have one moduleResolution option.
*/

let path = require("path");
const fs = require("fs");

const execSync = require("child_process").execSync;

function createCustomTsconfig(rootPath, entryFile, platforms) {
    const tsconfig = JSON.parse(fs.readFileSync(path.resolve(rootPath, "tsconfig.json"), "utf8"));
    tsconfig.compilerOptions.resolutionPlatforms = platforms;
    tsconfig.compilerOptions.noEmit = true;

    tsconfig.files = [path.resolve(rootPath, entryFile)];

    let cmdParams = "";

    for (var _ in tsconfig.compilerOptions) {
        if (typeof tsconfig.compilerOptions[_] == typeof true) {
            if (tsconfig.compilerOptions[_]) {
                cmdParams += `--${_} `;
            }
        } else if (typeof tsconfig.compilerOptions[_] == typeof "string") {
            cmdParams += `--${_} ${tsconfig.compilerOptions[_]} `;
        } else {
            cmdParams += `--${_} ${tsconfig.compilerOptions[_].join(",")} `;
        }
    }

    cmdParams += path.resolve(rootPath, entryFile);

    const cmd = `node ${path.resolve(rootPath, "../../node_modules/typescript/lib/tsc.js")} ${cmdParams}`;

    console.log(cmd);
    const output = execSync(cmd, { stdio: [0, 1, 2] });
}

createCustomTsconfig(path.resolve(__dirname, "../"), path.resolve(__dirname, "../index.mobile.ts"), ["ios", "native"]);
createCustomTsconfig(path.resolve(__dirname, "../"), path.resolve(__dirname, "../index.mobile.ts"), ["android", "native"]);

TypeScript Version: 2.7.1

Search Terms: react-native platform resolution

Code

A.ts

import { MyComponent } from 'B';
export class AComponent extends React.Component<{}, {}> {
  public render() {
    <BComponent foo={ 1 } />
  }
}

B.ts

export class BComponent extends React.Component<{ foo: number }, {}> {
  public render() { ... }
}

B.ios.ts

export class BComponent extends React.Component<{ foo: number, bar: string }, {}> {
  public render() { ... }
}

Expected behavior:
Type error when compiling for ios, since AComponent is not providing the required bar property.

Actual behavior:
tsc never checked B.ios.ts at all. -- So the code that gets included in the ios react-native bundle never gets checked as bundled.

Related Issues:
One issue this doesn't solve is cross package react-native platform resolution. If package C has A and B as above, but A exported B. Then A.d.ts would likely contain the incorrect information as its not clear
which version of B it would include. I think the next evolution of this change would be to have tsc output a separate A.ios.d.ts and A.android.d.ts when doing the platform specific builds. Thus ensuring that cross package type checking can also be done. This shouldn't be too hard to add, and would be completely additive to this change.

@DanielRosenwasser DanielRosenwasser added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Feb 14, 2018
@TheLarkInn
Copy link
Member

TheLarkInn commented Mar 13, 2018

For the sake of my own context. Is this issue specifically rooted in the fact that metro is using their non standard haste module system? Or is this not entirely related. Or better yet, would this issue still occur if you are packaging/bundling your app using webpack to bundle which actually respects NodeJS's / or ESM module resolution system.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Mar 13, 2018

This would still be based on Node's resolution strategy, but with platform-specific extensions in mind. It doesn't have any of the specific quirks of Haste, since that can usually be achieved via path mapping/aliasing.

@deepsweet
Copy link

Please consider using a more abstract extensions option or something rather than just the React Native particular case. In my project I have a "custom" .web.js along with .native.js, .android.js and .ios.js, and it's all solved gracefully in every tool around from Babel to Webpack only because there is a way to tweak extensions.

@tkow
Copy link

tkow commented Nov 9, 2018

This feature is really wanted for react-native users. Our team use .native.ts, and .web.ts postfix rules if
we use almost reusable modules have partial difference about web and native. Now, we must specify alias paths for importing file have either extensions because relative paths don't support changing extensions as mentioning in #8328. Because of that ,though bundle works in webpack configured resolve extensions, vscode doesn't analyze type unless using set compileOption's paths like "~ / *": ["./ *", "./ *.native", "./ *.ios", "./ *.android"], and even if exported file in same directory in which located the file import it , we must specify '~/dir1/dir2/dir3/.../target' the file point to target.native.ts, target.ios.ts or target.android.ts. I really have trouble that configuration always more complex as it is.

@vjsingh
Copy link

vjsingh commented Oct 9, 2019

Any updates on this? It seems Typescript doesn't support the ".native.js" and ".web.js" loading mechanism in react-native.

@anotherliam
Copy link

Just encountered this problem ourselves - VSCode is giving us suggestions based on our web components even in our Native code.

@vitorcamachoo
Copy link

I'm facing the same issue in our projects. We have a isomorphic application, and Typescript don't resolve ".web.js" extensions.

@fractalgelfo
Copy link

bump, would love to see this! has anybody found a workaround for compile-time resolution? I'm aware of this approach usingexport myLibrary = Platform.OS === 'web' ? require('web-version') : eval('require')('native-version'), but that's definitely not desirable!

@pandigita
Copy link

pandigita commented Dec 10, 2019

We're running into major problems in a react, react-native project that is supposed to share a lot of code and uses TypeScript. Because this feature is missing we're wondering if TS was the wrong decision. Did anyone manage to set TS up for this?

@fractalgelfo
Copy link

I wasn't able to set up TypeScript for this, per se, but I did come up with a webpack-based solution that is perfectly acceptable.

Here is my webpack config file as an example. It sets up a mocked_modules folder and adds aliases for the libraries that I needed to polyfill in order to work in react-native-web.

const path = require('path');
const createExpoWebpackConfigAsync = require('@expo/webpack-config');

module.exports = async function(env, argv) {
  const config = await createExpoWebpackConfigAsync(env, argv);
  // Customize the config before returning it.
  const addAlias = (
    moduleName,
    moduleDir = 'mocked_modules',
    moduleAlias = ''
  ) => {
    config.resolve.alias[moduleName] = path.resolve(
      __dirname,
      `./${moduleDir}/${moduleAlias || moduleName}`
    );
  };

  addAlias('react-native-email-link');
  addAlias('sentry-expo');
  addAlias('lottie-react-native');
  addAlias(
    '@use-expo/screen-orientation',
    'mocked_modules',
    'use-expo-screen-orientation'
  );
  addAlias('react-native');

  return config;
};

With this, when I run react-native on a mobile device, it will use the node_modules version of the package, and when I run react-native-web in the browser, it uses the mocked_modules version.

Consideration: This is more of a polyfill approach where the declarations & types of the modules stay the same between platforms. TypeScript still pulls the types from node_modules.

@arlyon
Copy link

arlyon commented Jun 9, 2020

Solution in the mean time is to include a ".d.ts" declaration file with the same name to inform typescript of the common interface

@jehartzog
Copy link

jehartzog commented Oct 27, 2020

I got TypeScript to work in ideal way, where running for my /native project folder it looks up preferentially .native (or other variants). It's working nicely with vscode and all CLI tools.

tsconfig.json

  "compilerOptions": {
    "paths": {
      "*": ["*.ios", "*.android", "*.native", "*/index.ios", "*/index.android", "*/index.native", "*"]
    },

The extra index ones were necessary if you are using the modules/foo -> modules/foo/index default mapping.

Just cause if you're looking at this, chances are you are fighting with eslint to make this work, just make sure the order you put your extensions is correct:

npx eslint ./src --ext .android.ts,.ios.ts,.native.ts,.android.tsx,.ios.tsx,.native.tsx,.js,.jsx,.ts,.tsx 

@jehartzog
Copy link

I forgot to add, I also had to create modules/foo/index.native.ts files when I needed to do different type exports.

modules/foo/index.native.ts

import Text, {TextProps} from './Text.native';
export {TextProps};
export default Text;

@dcalhoun
Copy link

dcalhoun commented Jun 8, 2021

@acoates-ms thanks for creating this proposal. 🙇🏻

I noticed there hasn't been much activity on this topic. @DanielRosenwasser do you know if there is any specific work needed to move this proposal forward? Anything the community can contribute? Thank you!

@afoxman
Copy link
Contributor

afoxman commented Mar 9, 2022

Based on @DanielRosenwasser's recent meeting notes, I've put together an implementation of this proposal which achieves the desired outcome for react-native while being more generally useful in other contexts. I'll be submitting a PR shortly and tying it to this proposal.

Detailed explanation is here: https://github.com/afoxman/rnts-example#react-native-platform-extensions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet