Skip to content

Implement container for the inspected application and components#15

Open
shining-mind wants to merge 12 commits intomainfrom
refactor
Open

Implement container for the inspected application and components#15
shining-mind wants to merge 12 commits intomainfrom
refactor

Conversation

@shining-mind
Copy link
Contributor

No description provided.

@shining-mind shining-mind requested a review from ItMaga May 3, 2024 11:36
Comment on lines +151 to +152
// eslint-disable-next-line @typescript-eslint/no-floating-promises
ctx.setMod(name, value);
Copy link
Member

Choose a reason for hiding this comment

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

может тут void, раз игнорируем?

Copy link
Member

Choose a reason for hiding this comment

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

А не странно, что у нас engines/loopback-engine.ts, который экспортирует evaluateEngine и disposeEngine? Как будто движки в движке, может сделать просто engines/loopback.ts?

import { devtoolsEval } from 'core/browser-api';
import type { DisposeEngine, EvaluateEngine } from 'core/inspect/devtools-handle/interface';

// eslint-disable-next-line @v4fire/require-jsdoc
Copy link
Member

Choose a reason for hiding this comment

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

Может тут добавить ссылку на DevtoolsHandle.evaluate? Чтобы было более очевидно, что именно переопределяем в слое

TreeItem[] children
}

class InspectedApp {
Copy link
Member

Choose a reason for hiding this comment

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

А подскажи, пожалуйста, я правильно понимаю, что этот класс будет отвечать за общение между бэкендом и конечным клиентом?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Да, задумка была такая: сделать абстракцию над исследуемым приложением, по аналогии как Page в Playwright.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants