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

Finetune install connect test #14518

Merged
merged 3 commits into from
Sep 25, 2024
Merged

Finetune install connect test #14518

merged 3 commits into from
Sep 25, 2024

Conversation

mroz22
Copy link
Contributor

@mroz22 mroz22 commented Sep 25, 2024

there is a test that connect is installable from npm, types work, and we didn't screw up.

the problem is that it is now falling in CI so we might have screwed up actually.

to get this information earlier, I suggest extending it also to beta channel.

the problem is that I don't understand what is going on there right now. Maybe @martykan got some clue

image

@mroz22
Copy link
Contributor Author

mroz22 commented Sep 25, 2024

it looks like it can't resolve imports from sinclair. It reminded me of this script that is replacing imports from monorepo with actual ones https://github.com/trezor/trezor-suite/blob/develop/scripts/replace-imports.sh#L19 but it shouldn't be the case here

also .mjs is suspicious. could be that I just need some additional configuration

@martykan
Copy link
Member

martykan commented Sep 25, 2024

Yeah looks like we have a problem.
Replacing @sinclair/typebox/build/esm/index.mjs -> @sinclair/typebox in the build does solve the issue.
In the old version it was correctly importing as @sinclair/typebox. Not sure what changed.

There is just one more regarding USB
Screenshot 2024-09-25 at 09 41 05

@mroz22
Copy link
Contributor Author

mroz22 commented Sep 25, 2024

usb lib depends on w3c-web-usb types. so we probably need to make sure that it is always installed

image

this should help I think 74c7bca

@martykan martykan force-pushed the finetune-install-connect-test branch 2 times, most recently from b355ef3 to d80609e Compare September 25, 2024 11:14
@martykan martykan force-pushed the finetune-install-connect-test branch from d80609e to 3f1f423 Compare September 25, 2024 11:15
@@ -91,6 +91,7 @@
"devDependencies": {
"@trezor/trezor-user-env-link": "workspace:*",
"@types/karma": "^6.3.8",
"@types/w3c-web-usb": "^1.0.10",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shows up as an unused dependency, which makes sense since it's used in @packages/transport not @packages/connect

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and it shouldn't be needed since it is listed in usb package

image

@mroz22 mroz22 force-pushed the finetune-install-connect-test branch from 3f1f423 to 5e7cf7f Compare September 25, 2024 12:37
@mroz22 mroz22 force-pushed the finetune-install-connect-test branch from 5e7cf7f to 1c267f3 Compare September 25, 2024 13:06
@mroz22 mroz22 marked this pull request as ready for review September 25, 2024 13:08
@mroz22 mroz22 enabled auto-merge (rebase) September 25, 2024 13:27
@mroz22 mroz22 merged commit 98d6437 into develop Sep 25, 2024
95 of 98 checks passed
@mroz22 mroz22 deleted the finetune-install-connect-test branch September 25, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants