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

Модуль с функциями отправки событий статистики в счетчики #1

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

Conversation

natalya-z
Copy link
Collaborator

No description provided.

curly: ['error', 'all'],
quotes: ['error', 'single'],
'no-alert': 'error',
'no-console': 'warn',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Может вернуть error? В либе думаю не нужны лишние выводы в консоль

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

нужно для вывода ошибок в консоль об отправки статистики

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

добавить флаг debug в init, при true - выводить ошибки в консоль

Comment on lines 19 to 22
* @property {string=} event - id события
* @property {string=} title - название действия по событию
* @property {string=} category - категория события
* @property {Object=} rest - дополнительные параметры
Copy link
Collaborator

Choose a reason for hiding this comment

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

Разве они устанавливаются по умолчанию?

Comment on lines +79 to +81
* @property {ActionType=} action - event, title события
* @property {string=} category - категория события
* @property {Object=} rest - дополнительные параметры
Copy link
Collaborator

Choose a reason for hiding this comment

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

Тоже вроде по умолчанию не устанавливаются

const pages = {
start: {
path: '/start',
title: categories.start,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Тут наверное лучше читаемое название указать

CHANGELOG.md Outdated
@@ -0,0 +1,2 @@
### v1.0.0
- [+] статистика

Choose a reason for hiding this comment

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

не очень подробно...

@@ -0,0 +1,77 @@
import 'whatwg-fetch';

Choose a reason for hiding this comment

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

а в чем преимущества такого фетча?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

это полифилл для настоящего fetch. он есть в новых браузерах (https://caniuse.com/fetch), мне кажется, пользователи, у которых старые версии все еще могут существовать:
Снимок экрана 2022-02-17 в 22 55 29

json: '',
},
],
v: '5.124',

Choose a reason for hiding this comment

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

вроде актуальная версия повыше уже

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

я не уверена, какие в актуальной версии поля в закрытом вкшном методе статы(

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ввела переменную version, куда можно передавать версию api

access_token: access_token_for_vkstats,
},
});
} catch (e: any) {

Choose a reason for hiding this comment

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

мб типизируем тут ошибку?

Copy link
Collaborator Author

@natalya-z natalya-z Feb 17, 2022

Choose a reason for hiding this comment

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

ошибка может быть либо unknown, либо any, так задумано ts.
microsoft/TypeScript#20024
можно написать:

catch (e: unknown) {
    const error = e as ErrorData;
}

* Top Mail
* KTS статистика
* ВК ститистика
Copy link
Collaborator

Choose a reason for hiding this comment

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

статистика

* VK Retargeting
* Snitch
* Писели
Copy link
Collaborator

Choose a reason for hiding this comment

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

Пиксели?

userId: window.user_id,
});
```

### метод setUserId
```sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

Тут какой-то пример должен быть?

Comment on lines +58 to +59
SEARCH: KTS_DATA_SAVE?.KTS_TOKEN || '',
KTS_TOKEN: KTS_EVENT_REGISTER?.SEARCH || '',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Не поняла, почему токен в search, а search в токене

Comment on lines +46 to +49
KTS_PROJECT_NAME &&
KTS_TOKEN &&
!SEARCH &&
KTS_STATS_URL &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

Вообще может в __params__ такую же структуру оставить, как в аргументах init? Тип поля KTS_DATA_SAVE и KTS_EVENT_REGISTER, и они либо null, либо со всей нужной инфой. Чтобы не путаться в этих условиях

headers,
});
} catch (e) {
console.error('stat kts error', e);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ошибки в итоге будут в консоли?

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.

5 participants