Register returns any, breaking typescript's type check

[demedos]

Describe the bug The register function returns any, breaking typescript's type check.

Code snippet

// MyComponent/index.ts
import { register } from 'react-native-bundle-splitter';

export default register({ loader: () => import('./MyComponent') });

// MyComponent/MyComponent.tsx
import React from 'react';
import { View } from 'react-native';

type Props = {
  isActive: boolean;
};

export default (_props: Props) => {
  return <View />;
};

// Screens/index.tsx
import React from 'react';
import MyComponent from '../MyComponent';

export default () => {
  // The following code should fail the type check
  return <MyComponent wrongProp1={[]} wrongProp2={false} />;
};

Repo for reproducing https://github.com/demedos/bundle-split-repro

To Reproduce Steps to reproduce the behavior:

1. Add the bundle split to a component
2. Import the component into another one
3. Pass the wrong props

Expected behavior Typescript's type check (yarn tsc) should throw an error

Screenshots If applicable, add screenshots to help explain your problem.

Smartphone (please complete the following information):

• Desktop OS: NA
• Device: NA
• OS: NA
• JS engine: NA
• Library version: 2.1.0

Additional context Add any other context about the problem here.

[kirillzyusko]

Hi @demedos

Thank you for raising the issue. I think I've been trying to specify correct types, though it wasn't easy 😅

As far as I remember we can not inherit type from import function (correct me if I'm wrong). The other possible way would be making register function to accept generic T. But I suppose it could bring more issues because you would need to define type Props at the same level with register function, which is not good from code perspective (or store in separate file? It's better but far from ideal variant). I think that was the reason why I made it any (I know it's very bad 😅)

Do you have any ideas how to fix it? Or could you at least share your thoughts?

[demedos]

As far as I know there's not way to infer the type from the import function.

Something like this would solve the problem, but this would also load the file immediately, making the bundle split useless. Also, it's just a type cast, which is not ideal as we are forcing the return type. Its usage would be like so:

// MyComponent/index.ts
import { register } from 'react-native-bundle-splitter';
import MyComponent from './MyComponent';

export default register<typeof MyComponent>({ loader: () => import('./MyComponent') });

We should definitely find a way to remove the any return type from the optimized function and let typescript infer it

[kirillzyusko]

@demedos don't you think, that the line

import MyComponent from './MyComponent';

will break a concept of lazy loading? You will have a direct reference to the file and it will be bundled with main part of the application.

The idea, that you suggest I described here:

The other possible way would be making register function to accept generic T. But I suppose it could bring more issues because you would need to define type Props at the same level with register function, which is not good from code perspective (or store in separate file? It's better but far from ideal variant). I think that was the reason why I made it any (I know it's very bad 😅)

I can add generic with default value to the code, but you as a developer will need to keep in mind, that you can not have direct references to file, otherwise it will break a concept of lazy initialization. Though you can always create file types.ts and put following content:

// types.ts
export type Props = {
  isActive: boolean;
};

And use it in next way:

// MyComponent/index.ts
import { FC } from "react";
import { register } from 'react-native-bundle-splitter';
import { Props } from './types';

export default register<FC<Props>>({ loader: () => import('./MyComponent') });

Will it work for you or not? Though, again, you need to be careful and always keep an eye on the content inside of packager.

Also I'm going to check similar packages for web and how they bring a support for TS.

[demedos]

Yes, it would break the lazy loading, that's what I was saying

[...] this would also load the file immediately, making the bundle split useless.

I see no other solution than to use a generic and manually move all the props in their own files. The developer should then check if it is necessary to use the FC interface over ReactComponent

[kirillzyusko]

this would also load the file immediately, making the bundle split useless.

@demedos I don't know how I missed it 🤦‍♂️

Yes, it sounds like a potential option to add it. I think I will investigate it a little bit more, and if I don't find any better solutions, then I will implement these changes and will publish a new release!

I hope within several days it should be done 😊

[demedos]

Sounds good, thank you 🙂 The only downside of this is that passing FC<Props> as a generic may be wrong, because even if the source component is a FC, optimized always returns a new pure component. Nonetheless, it should work without any problem

[kirillzyusko]

@demedos I've created a MR with fix: https://github.com/kirillzyusko/react-native-bundle-splitter/pull/34

Could you have a look and say whether it fixes your problem or not?

[demedos]

Yes, it is indeed a great improvement. I use react-redux's ConnectedProps type to infer the action and properties injected in the component through the connect function; this means that when moving the Props type to its own file, the connector must imported from the component file itself. Do you think that this can affect the bundle split? See an example below:

// MyComponent.tsx

import { connect, ConnectedProps } from 'react-redux';

export const connector = connect(mapStateToProps, mapDispatchToProps);

// types.ts

import { connector } from './MyComponent';

export type Props = ConnectedProps<typeof connector>

// index.ts

import { Props } from './types';

export default register<Props>({ loader: () => import('./MyComponent') });

[kirillzyusko]

@demedos I bet on "yes", it will affect and the file will not be lazily loaded. Though, you can try to verify it by calling investigate method. It returns all files, which you will have in your main bundle (these files will be loaded at the start of the application).

We also are using redux in combination with this library. But almost all the time all injected Props by redux we describe manually, so it wasn't an issue for us.

[kirillzyusko]

Closed, because https://github.com/kirillzyusko/react-native-bundle-splitter/pull/34 has been merged.