trezor / trezor-suite

Trezor Suite Monorepo
https://trezor.io/trezor-suite
Other
723 stars 251 forks source link

@trezor/connect-build: consider adding package #5305

Open mroz22 opened 2 years ago

mroz22 commented 2 years ago

dev webpack configs in @treozr/connect-web

trezor-suite/packages/connect-web/webpack/dev.webpack.config.ts

and @trezor/connect-explorer

trezor-suite/packages/connect-explorer/webpack/dev.webpack.config.ts

contain imports from other packages (popup, iframe). Which is causing type-check to emit the following error: Projects must list all files or use an 'include' pattern.ts(6307)

This could be fixed by adding @trezor/connect-popup and @trezor/connect-iframe into tsonfig.json but this is doable only for @trezor/connect-explorer. On the other, @trezor/connect-web might not have these dependencies as adding them into tsconfig.json requires adding them to package.json dependencies also and @trezor/connect-web should not have @trezor/connect-iframe listed here (it wouldn't work for implementators)

Nodonisko commented 2 years ago

Not sure if I understand problem, you don't want to add these dependencies because they should be here only for dev and not for production? Why not adding them as devDependecies.

szymonlesisz commented 1 year ago

i believe this is similar to a "very hacky import solution" in suite-common/intl-types package which after closer inspection is not working either. type-check job is not returning errors even if the errors are there.

Example: change import to get some obvious error: https://github.com/trezor/trezor-suite/blob/develop/suite-common/intl-types/src/types.ts

import { MessageDescriptor } from 'react-intl2';

vscode is showing error but

yarn workspace @suite-common/intl-types type-check

is not

Nodonisko commented 1 year ago

I did some investigation and I definitely doesn't encourage anyone to change rootDir because all urls in tsconfig are resolved agains that so it will change also things like outputDir, references etc and TS will behave very strangely then like no throwing errors when there is bad errors and maybe some other weird stuff.

One "less hacky" solution is basically to use require instead of import:

// eslint-disable-next-line @typescript-eslint/no-var-requires
const iframe = require('../../connect-iframe/webpack/prod.webpack.config');
const popup = require('../../connect-popup/webpack/prod.webpack.config');

This will work because TS type this import as any which I think is not issue here.

Another solution could be to just use .js for these files so TS will ignore them at all.

Another solution could be to exclude these files completely in tsconfig.json and keep them in .ts format (I think autocomplete will still work here). If you really want them checked you can create some very simple tsconfig.dev.json file that will include only these files.

@mroz22 @szymonlesisz