Skip to content
This repository has been archived by the owner on Oct 16, 2024. It is now read-only.

Remove logs from UI library #21

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Remove logs from UI library #21

wants to merge 4 commits into from

Conversation

sshmaxime
Copy link
Contributor

πŸ“ Description

Temporarily removing logs coming from @ledgerhq/react-ui for test and client/server components. It's much easier to develop.

❓ Context

  • Linked resource(s): ``

πŸ“Έ Demo

πŸš€ Expectations to reach

Pull Requests must pass the CI and be internally validated in order to be merged.

@vercel
Copy link

vercel bot commented Sep 22, 2023

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
cex-deposit-live-app βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback Sep 22, 2023 2:28pm

/**
* @dev temporary removing some logs in client.
*/
require("../../../tools/removeLogs");
Copy link
Contributor

Choose a reason for hiding this comment

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

You can't use alias? :)

Copy link
Contributor Author

@sshmaxime sshmaxime Sep 22, 2023

Choose a reason for hiding this comment

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

Sadly no, it doesn't work in test ... Idk why. Look at my second commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

if you do

Suggested change
require("../../../tools/removeLogs");
import "@/tools/removeLogs"

This don't works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok when doing that I had an error saying that the file wasn't a module so I just added module.exports = {} and it worked. Cool, thanks for the heads up @mcayuelas-ledger !

jest.setup.js Outdated
Comment on lines 10 to 14
/**
* @dev temporary removing some logs.
*/
require("./tools/removeLogs");

Copy link
Member

Choose a reason for hiding this comment

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

We should not mix require and import. You can use an import here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch

Comment on lines +3 to +7
/**
* @dev temporary removing some logs in client.
*/
import("@/tools/removeLogs");

Copy link
Member

Choose a reason for hiding this comment

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

I think that we should parse errors only for tests. I'm not convinced doing that at runtime for the client.

Copy link
Member

Choose a reason for hiding this comment

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

Even for the server

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants