chore: convert bundle command to TS #695
Conversation
Quadriphobs1
requested review from
Esemesek,
grabbou and
thymikee
as code owners
thymikee
changed the title
| ...Server.DEFAULT_BUNDLE_OPTIONS, | ||
| ...requestOpts, | ||
| bundleType: 'todo', | ||
| }); | ||
|
|
||
| // When we're done saving bundle output and the assets, we're done. | ||
| // @ts-ignore |
There was a problem hiding this comment.
why ts ignore? can you leave a comment explaining?
There was a problem hiding this comment.
the value args.platform is actually seen as string | undefined however in the function saveAssets() it is required which causes type error... looking at the code it looks like the args.platform would be provided at all times which makes no sense to actually make it optional from the code as well which is why I suppress the warning but what do you think of it
There was a problem hiding this comment.
Definitely shouldn't be suppressed. This is possible failure point and we need to make sure args.platform is defined before passing it further
There was a problem hiding this comment.
I just checked in now the args.platform has a default value of ios which means making it options is actually not necessary
| */ | ||
|
|
||
| import {Config} from '@react-native-community/cli-types'; | ||
| import {CommandLineArgs} from './bundleCommandLineArgs'; | ||
| import buildBundle from './buildBundle'; | ||
| import bundleCommandLineArgs from './bundleCommandLineArgs'; |
There was a problem hiding this comment.
You can remove line 9 nad move the import here
| import bundleCommandLineArgs from './bundleCommandLineArgs'; | |
| import bundleCommandLineArgs, {CommandLineArgs} from './bundleCommandLineArgs'; |
| */ | ||
|
|
||
| import path from 'path'; | ||
| import type {PackagerAsset} from './assetPathUtils'; | ||
| import {PackagerAsset} from './assetPathUtils'; | ||
|
|
||
| import assetPathUtils from './assetPathUtils'; |
There was a problem hiding this comment.
please consolidate imports :)
| import assetPathUtils from './assetPathUtils'; | |
| import assetPathUtils, {PackagerAsset} from './assetPathUtils'; |
| import outputUnbundle from 'metro/src/shared/output/RamBundle'; | ||
|
|
||
| import {CommandLineArgs} from './bundleCommandLineArgs'; | ||
| import {withOutput as bundleWithOutput} from './bundle'; | ||
| import bundleCommandLineArgs from './bundleCommandLineArgs'; |
There was a problem hiding this comment.
| import bundleCommandLineArgs from './bundleCommandLineArgs'; | |
| import bundleCommandLineArgs, {CommandLineArgs} from './bundleCommandLineArgs'; |
| import saveAssets from './saveAssets'; | ||
| import loadMetroConfig from '../../tools/loadMetroConfig'; | ||
| // @ts-ignore FIXME after converting tools/loadMetroConfig | ||
| import loadMetroConfig from '../../tools/loadMetroConfig'; // eslint-disable-line import/namespace, import/default |
There was a problem hiding this comment.
you can remove the ignores after a rebase :)
|
Thank you! |
Summary:
Part of #683
Converted
commands/bundle/to typescriptTest Plan:
yarn test