Skip to content

Commit

Permalink
ref(feedback): Refactor Feedback types into @sentry/types and reduce …
Browse files Browse the repository at this point in the history
…the exported surface area (getsentry#11355)

You can read this commit-by-commit to watch the refactor unfold

The situation is: after
getsentry#11342 is merged, all
the code inside each of the
`packages/feedback/src/[core|modal|screenshot]` will be split up into
separate integrations. These integrations can't share the same
`src/types/*.ts` definitions anymore. Therefore type definitions will
need to live in @sentry/types instead.

This PR moves all the types, and since they'll be public now in that
package, i refactored things to reduce the public surface area and
improve names where possible.
The only non-type changes in here are where we move `createDialog.ts`
and `createInput.ts` into their respective `integration.ts` files, no
code changes at all.

Related to getsentry#11435
  • Loading branch information
ryan953 authored and cadesalaberry committed Apr 19, 2024
1 parent 661f9cc commit c175d14
Show file tree
Hide file tree
Showing 25 changed files with 346 additions and 362 deletions.
9 changes: 6 additions & 3 deletions packages/feedback/src/core/createMainStyles.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { FeedbackInternalOptions } from '@sentry/types';
import { DOCUMENT } from '../constants';
import type { FeedbackTheme, FeedbackThemes } from '../types';

function getThemedCssVariables(theme: FeedbackTheme): string {
function getThemedCssVariables(theme: FeedbackInternalOptions['themeLight']): string {
return `
--background: ${theme.background};
--background-hover: ${theme.backgroundHover};
Expand Down Expand Up @@ -39,7 +39,10 @@ function getThemedCssVariables(theme: FeedbackTheme): string {
/**
* Creates <style> element for widget actor (button that opens the dialog)
*/
export function createMainStyles(colorScheme: 'system' | 'dark' | 'light', themes: FeedbackThemes): HTMLStyleElement {
export function createMainStyles(
colorScheme: 'system' | 'dark' | 'light',
themes: Pick<FeedbackInternalOptions, 'themeLight' | 'themeDark'>,
): HTMLStyleElement {
const style = DOCUMENT.createElement('style');
style.textContent = `
:host {
Expand Down
38 changes: 13 additions & 25 deletions packages/feedback/src/core/integration.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { getClient } from '@sentry/core';
import type { Integration, IntegrationFn } from '@sentry/types';
import type {
FeedbackDialog,
FeedbackInternalOptions,
FeedbackModalIntegration,
FeedbackScreenshotIntegration,
IntegrationFn,
} from '@sentry/types';
import { isBrowser, logger } from '@sentry/utils';
import {
ACTOR_LABEL,
Expand All @@ -17,34 +23,16 @@ import {
SUBMIT_BUTTON_LABEL,
SUCCESS_MESSAGE_TEXT,
} from '../constants';
import type { IFeedbackModalIntegration } from '../modal/integration';
import type { IFeedbackScreenshotIntegration } from '../screenshot/integration';
import type {
Dialog,
FeedbackInternalOptions,
OptionalFeedbackConfiguration,
OverrideFeedbackConfiguration,
} from '../types';
import { DEBUG_BUILD } from '../util/debug-build';
import { isScreenshotSupported } from '../util/isScreenshotSupported';
import { mergeOptions } from '../util/mergeOptions';
import { Actor } from './components/Actor';
import { createMainStyles } from './createMainStyles';
import { sendFeedback } from './sendFeedback';
import type { OptionalFeedbackConfiguration, OverrideFeedbackConfiguration } from './types';

type Unsubscribe = () => void;

interface PublicFeedbackIntegration {
attachTo: (el: Element | string, optionOverrides: OverrideFeedbackConfiguration) => () => void;
createWidget: (optionOverrides: OverrideFeedbackConfiguration & { shouldCreateActor?: boolean }) => Promise<Dialog>;
getWidget: () => Dialog | null;
remove: () => void;
openDialog: () => void;
closeDialog: () => void;
removeWidget: () => void;
}
export type IFeedbackIntegration = Integration & PublicFeedbackIntegration;

/**
* Allow users to capture user feedback and send it to Sentry.
*/
Expand Down Expand Up @@ -148,13 +136,13 @@ export const feedbackIntegration = (({
return _shadow as ShadowRoot;
};

const _loadAndRenderDialog = async (options: FeedbackInternalOptions): Promise<Dialog> => {
const _loadAndRenderDialog = async (options: FeedbackInternalOptions): Promise<FeedbackDialog> => {
const client = getClient(); // TODO: getClient<BrowserClient>()
if (!client) {
throw new Error('Sentry Client is not initialized correctly');
}
const modalIntegration = client.getIntegrationByName<IFeedbackModalIntegration>('FeedbackModal');
const screenshotIntegration = client.getIntegrationByName<IFeedbackScreenshotIntegration>('FeedbackScreenshot');
const modalIntegration = client.getIntegrationByName<FeedbackModalIntegration>('FeedbackModal');
const screenshotIntegration = client.getIntegrationByName<FeedbackScreenshotIntegration>('FeedbackScreenshot');
const screenshotIsSupported = isScreenshotSupported();

// START TEMP: Error messages
Expand Down Expand Up @@ -196,7 +184,7 @@ export const feedbackIntegration = (({
throw new Error('Unable to attach to target element');
}

let dialog: Dialog | null = null;
let dialog: FeedbackDialog | null = null;
const handleClick = async (): Promise<void> => {
if (!dialog) {
dialog = await _loadAndRenderDialog({
Expand Down Expand Up @@ -264,7 +252,7 @@ export const feedbackIntegration = (({
/**
* Creates a new widget. Accepts partial options to override any options passed to constructor.
*/
async createWidget(optionOverrides: OverrideFeedbackConfiguration = {}): Promise<Dialog> {
async createWidget(optionOverrides: OverrideFeedbackConfiguration = {}): Promise<FeedbackDialog> {
return _loadAndRenderDialog(mergeOptions(_options, optionOverrides));
},

Expand Down
11 changes: 5 additions & 6 deletions packages/feedback/src/core/sendFeedback.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import { createAttachmentEnvelope, createEventEnvelope, getClient, withScope } from '@sentry/core';
import type { FeedbackEvent, TransportMakeRequestResponse } from '@sentry/types';
import type { FeedbackEvent, SendFeedback, SendFeedbackParams } from '@sentry/types';
import { getLocationHref } from '@sentry/utils';
import { FEEDBACK_API_SOURCE, FEEDBACK_WIDGET_SOURCE } from '../constants';
import type { SendFeedbackOptions, SendFeedbackParams } from '../types';
import { prepareFeedbackEvent } from '../util/prepareFeedbackEvent';

/**
* Public API to send a Feedback item to Sentry
*/
export async function sendFeedback(
export const sendFeedback: SendFeedback = (
{ name, email, message, attachments, source = FEEDBACK_API_SOURCE, url = getLocationHref() }: SendFeedbackParams,
{ includeReplay = true }: SendFeedbackOptions = {},
): Promise<TransportMakeRequestResponse> {
{ includeReplay = true } = {},
) => {
if (!message) {
throw new Error('Unable to submit feedback with empty message');
}
Expand Down Expand Up @@ -97,7 +96,7 @@ export async function sendFeedback(
throw error;
}
});
}
};

/*
* For reference, the fully built event looks something like this:
Expand Down
19 changes: 19 additions & 0 deletions packages/feedback/src/core/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import type { FeedbackInternalOptions } from '@sentry/types';

/**
* Partial configuration that overrides default configuration values
*
* This is the config that gets passed into the integration constructor
*/
export interface OptionalFeedbackConfiguration
extends Omit<Partial<FeedbackInternalOptions>, 'themeLight' | 'themeDark'> {
themeLight?: Partial<FeedbackInternalOptions['themeLight']>;
themeDark?: Partial<FeedbackInternalOptions['themeLight']>;
}

/**
* Partial configuration that overrides default configuration values
*
* This is the config that gets passed into the integration constructor
*/
export type OverrideFeedbackConfiguration = Omit<Partial<FeedbackInternalOptions>, 'themeLight' | 'themeDark'>;
3 changes: 0 additions & 3 deletions packages/feedback/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,3 @@ export { feedbackIntegration } from './core/integration';
export { feedbackModalIntegration } from './modal/integration';
export { getFeedback } from './core/getFeedback';
export { feedbackScreenshotIntegration } from './screenshot/integration';

export type { OptionalFeedbackConfiguration } from './types';
export type { IFeedbackIntegration } from './core/integration';
2 changes: 1 addition & 1 deletion packages/feedback/src/modal/components/DialogContainer.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import type { FeedbackFormData } from '@sentry/types';
// biome-ignore lint/nursery/noUnusedImports: reason
import { Fragment, h } from 'preact'; // eslint-disable-line @typescript-eslint/no-unused-vars
import type { VNode } from 'preact';
import { useCallback, useMemo, useState } from 'preact/hooks';
import { SUCCESS_MESSAGE_TIMEOUT } from '../../constants';
import type { FeedbackFormData } from '../../types';
import { DialogContent } from './DialogContent';
import { DialogHeader } from './DialogHeader';
import type { Props as HeaderProps } from './DialogHeader';
Expand Down
2 changes: 1 addition & 1 deletion packages/feedback/src/modal/components/DialogHeader.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type { FeedbackInternalOptions } from '@sentry/types';
// biome-ignore lint/nursery/noUnusedImports: reason
import { h } from 'preact'; // eslint-disable-line @typescript-eslint/no-unused-vars
import type { VNode } from 'preact';
import { useMemo } from 'preact/hooks';
import type { FeedbackInternalOptions } from '../../types';
import type { Props as LogoProps } from './SentryLogo';
import { SentryLogo } from './SentryLogo';

Expand Down
31 changes: 16 additions & 15 deletions packages/feedback/src/modal/components/Form.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import type {
FeedbackFormData,
FeedbackInternalOptions,
FeedbackScreenshotIntegration,
SendFeedback,
} from '@sentry/types';
import { logger } from '@sentry/utils';
// biome-ignore lint/nursery/noUnusedImports: reason
import { h } from 'preact'; // eslint-disable-line @typescript-eslint/no-unused-vars
import type { JSX, VNode } from 'preact';
import { useCallback, useState } from 'preact/hooks';
import { FEEDBACK_WIDGET_SOURCE } from '../../constants';
import type {
FeedbackFormData,
FeedbackInternalOptions,
ScreenshotInput,
SendFeedbackOptions,
SendFeedbackParams,
} from '../../types';
import { DEBUG_BUILD } from '../../util/debug-build';
import { getMissingFields } from '../../util/validate';

Expand All @@ -34,10 +33,10 @@ export interface Props
defaultEmail: string;
defaultName: string;
onFormClose: () => void;
onSubmit: (data: SendFeedbackParams, options?: SendFeedbackOptions) => void;
onSubmit: SendFeedback;
onSubmitSuccess: (data: FeedbackFormData) => void;
onSubmitError: (error: Error) => void;
screenshotInput: ScreenshotInput | undefined;
screenshotInput: ReturnType<FeedbackScreenshotIntegration['createInput']> | undefined;
}

function retrieveStringValue(formData: FormData, key: string): string {
Expand Down Expand Up @@ -74,8 +73,8 @@ export function Form({
const [error, setError] = useState<null | string>(null);

const [showScreenshotInput, setShowScreenshotInput] = useState(false);
const ScreenshotInput = screenshotInput && screenshotInput.input;
const includeScreenshotValue = ScreenshotInput && showScreenshotInput;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const ScreenshotInputComponent: any = screenshotInput && screenshotInput.input;

const [screenshotError, setScreenshotError] = useState<null | Error>(null);
const onScreenshotError = useCallback((error: Error) => {
Expand Down Expand Up @@ -112,7 +111,7 @@ export function Form({
return;
}
const formData = new FormData(e.target);
const attachment = await (includeScreenshotValue ? screenshotInput.value() : undefined);
const attachment = await (screenshotInput && showScreenshotInput ? screenshotInput.value() : undefined);
const data: FeedbackFormData = {
name: retrieveStringValue(formData, 'name'),
email: retrieveStringValue(formData, 'email'),
Expand All @@ -134,12 +133,14 @@ export function Form({
// pass
}
},
[includeScreenshotValue, onSubmitSuccess, onSubmitError],
[screenshotInput && showScreenshotInput, onSubmitSuccess, onSubmitError],
);

return (
<form class="form" onSubmit={handleSubmit}>
{includeScreenshotValue ? <ScreenshotInput onError={onScreenshotError} /> : null}
{ScreenshotInputComponent && showScreenshotInput ? (
<ScreenshotInputComponent onError={onScreenshotError} />
) : null}

<div class="form__right">
<div class="form__top">
Expand Down Expand Up @@ -192,7 +193,7 @@ export function Form({
/>
</label>

{ScreenshotInput ? (
{ScreenshotInputComponent ? (
<label for="screenshot" class="form__label">
<span class="form__label__text">Screenshot</span>

Expand Down
2 changes: 1 addition & 1 deletion packages/feedback/src/modal/components/SentryLogo.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { FeedbackInternalOptions } from '@sentry/types';
import { DOCUMENT } from '../../constants';
import type { FeedbackInternalOptions } from '../../types';
import { setAttributesNS } from '../../util/setAttributesNS';

const XMLNS = 'http://www.w3.org/2000/svg';
Expand Down
95 changes: 0 additions & 95 deletions packages/feedback/src/modal/createDialog.tsx

This file was deleted.

22 changes: 0 additions & 22 deletions packages/feedback/src/modal/integration.ts

This file was deleted.

Loading

0 comments on commit c175d14

Please sign in to comment.