Browse Source

ref(deviceName): update ios-device-name package to only use a small subset of device information (#32690)

* ref(deviceName): split load fn

* ref(deviceName): remove dead comment

* fix(tsc): use proper imports

* fix(tsc): fix test

* feat(ios-device-list): extract names

* ref(deviceName): only use what we need

* ref(deviceName): only use what we need

* ref(deviceName): add post-install.ts

* test

* Revert "test"

This reverts commit a3b0edc212bcc801ab0239a80c2581f66db1d9a5.

* ref(deviceName): add node_env reference

* ci(install): test echo pwd

* empty commit to trigger ci

* fix(ci): try listing dir

* fix(ci): try listing dir

* fix(devicename): hardcode ios device list

* test(eventTags): use mapper

* ref(devicelist): run as part of build step

* ref(devicelist): run prettier so it doesnt cause git diffs

* ref(devicelist): fix ts error

* ref(devicelist): check queue

* ref(devicelist): remove import side effect

* fix(deps): move prettier to build dep

* fix(gitignore): ignore auto generated file
Jonas 3 years ago
parent
commit
911b625b4f

+ 1 - 1
.github/workflows/scripts/monitor-typescript.ts

@@ -2,7 +2,7 @@
 import '@sentry/tracing';
 
 // eslint-disable-next-line import/no-nodejs-modules
-import * as fs from 'fs';
+import fs from 'fs';
 
 import * as Sentry from '@sentry/node';
 

+ 1 - 0
.gitignore

@@ -23,6 +23,7 @@ sentry-package.json
 /env
 /tmp
 node_modules
+/static/app/constants/ios-device-list.tsx
 /src/sentry/assets.json
 /src/sentry/static/version
 /src/sentry/static/sentry/dist/

+ 1 - 1
package.json

@@ -105,6 +105,7 @@
     "pegjs-loader": "^0.5.6",
     "platformicons": "^4.3.0",
     "po-catalog-loader": "2.0.0",
+    "prettier": "^2.6.0",
     "prism-sentry": "^1.0.2",
     "process": "^0.11.10",
     "prop-types": "^15.8.1",
@@ -172,7 +173,6 @@
     "jest-sentry-environment": "^1.5.0",
     "postcss-jsx": "0.36.4",
     "postcss-syntax": "0.36.2",
-    "prettier": "2.6.0",
     "react-refresh": "0.11.0",
     "react-select-event": "5.3.0",
     "react-test-renderer": "17.0.2",

+ 101 - 0
scripts/extract-ios-device-names.ts

@@ -0,0 +1,101 @@
+/* eslint-env node */
+import path from 'path';
+import fs from 'fs';
+
+import prettier from 'prettier';
+
+//joining path of directory
+const outputPath = path.join(__dirname, '../static/app/constants/ios-device-list.tsx');
+const directoryPath = path.join(__dirname, '../node_modules/ios-device-list/');
+
+async function getDefinitionFiles(): Promise<string[]> {
+  const files: string[] = [];
+
+  const maybeJSONFiles = await fs.readdirSync(directoryPath);
+
+  //listing all files using forEach
+  maybeJSONFiles.forEach(file => {
+    if (!file.endsWith('.json') || file === 'package.json') return;
+
+    files.push(path.join(path.resolve(directoryPath), file));
+  });
+
+  return files;
+}
+
+type Generation = string;
+type Identifier = string;
+
+type Mapping = Record<Identifier, Generation>;
+
+async function collectDefinitions(files: string[]): Promise<Mapping> {
+  const definitions: Mapping = {};
+
+  const queue = [...files];
+
+  while (queue.length > 0) {
+    const file = queue.pop();
+
+    if (!file) {
+      throw new Error('Empty queue');
+    }
+
+    const contents = fs.readFileSync(file, 'utf-8');
+    const content = JSON.parse(contents);
+
+    if (typeof content?.[0]?.Identifier === 'undefined') {
+      continue;
+    }
+
+    for (let i = 0; i < content.length; i++) {
+      definitions[content[i].Identifier] = content[i].Generation;
+    }
+  }
+
+  return definitions;
+}
+
+const HEADER = `
+// THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
+// generated using scripts/extract-ios-device-names.ts as part of build step.
+// the purpose of the script is to extract only the iOS information that Sentry cares about
+// and discard the rest of the JSON so we do not end up bloating bundle size.
+`;
+
+const template = (contents: string) => {
+  return `
+      ${HEADER}
+      const iOSDeviceMapping: Record<string, string> = ${contents}
+
+      export {iOSDeviceMapping}
+  `;
+};
+
+const formatOutput = async (unformatted: string) => {
+  const config = await prettier.resolveConfig(outputPath);
+  if (config) {
+    return prettier.format(unformatted, config);
+  }
+
+  return unformatted;
+};
+
+export async function extractIOSDeviceNames() {
+  try {
+    if (fs.statSync(outputPath)) {
+      // Out with the old, in with the new
+      fs.unlinkSync(outputPath);
+    }
+  } catch (e) {
+    // File does not exists, carry along
+  }
+
+  const files = await getDefinitionFiles();
+  const definitions = await collectDefinitions(files);
+  const formatted = await formatOutput(
+    template(JSON.stringify(definitions, undefined, 2))
+  );
+
+  fs.writeFileSync(outputPath, formatted);
+  console.log('✅ Regenerated ios-device-list.tsx');
+}

+ 5 - 45
static/app/components/deviceName.tsx

@@ -1,14 +1,8 @@
-import * as React from 'react';
-import * as Sentry from '@sentry/react';
+import {useMemo} from 'react';
 
-// Self reference to the module, so that we can mock a failed import in a test.
-import * as selfModule from 'sentry/components/deviceName';
-import {IOSDeviceList} from 'sentry/types/iOSDeviceList';
+import {iOSDeviceMapping} from 'sentry/constants/ios-device-list';
 
-export function deviceNameMapper(
-  model: string | undefined,
-  module: IOSDeviceList | null
-): string | null {
+export function deviceNameMapper(model: string | undefined): string | null {
   // If we have no model, render nothing
   if (typeof model !== 'string') {
     return null;
@@ -21,17 +15,10 @@ export function deviceNameMapper(
 
   const [identifier, ...rest] = model.split(' ');
 
-  const modelName = module.generationByIdentifier(identifier);
+  const modelName = iOSDeviceMapping[identifier];
   return modelName === undefined ? model : `${modelName} ${rest.join(' ')}`;
 }
 
-export async function loadDeviceListModule(platform: 'iOS') {
-  if (platform !== 'iOS') {
-    Sentry.captureException('DeviceName component only supports iOS module');
-  }
-  return import('ios-device-list');
-}
-
 interface DeviceNameProps {
   value: string;
   children?: (name: string) => React.ReactNode;
@@ -42,34 +29,7 @@ interface DeviceNameProps {
  * This asynchronously loads the ios-device-list library because of its size
  */
 function DeviceName({value, children}: DeviceNameProps): React.ReactElement | null {
-  const [deviceList, setDeviceList] = React.useState<IOSDeviceList | null>(null);
-
-  React.useEffect(() => {
-    let didUnmount = false;
-
-    selfModule
-      .loadDeviceListModule('iOS')
-      .then(module => {
-        // We need to track component unmount so we dont try and setState on an unmounted component
-        if (didUnmount) {
-          return;
-        }
-
-        setDeviceList(module);
-      })
-      .catch(() => {
-        Sentry.captureException('Failed to load ios-device-list module');
-      });
-
-    return () => {
-      didUnmount = true;
-    };
-  }, []);
-
-  const deviceName = React.useMemo(
-    () => deviceNameMapper(value, deviceList),
-    [value, deviceList]
-  );
+  const deviceName = useMemo(() => deviceNameMapper(value), [value]);
 
   return deviceName ? (
     <span data-test-id="loaded-device-name">

+ 6 - 50
static/app/components/group/tagDistributionMeter.tsx

@@ -1,9 +1,8 @@
 import {Component} from 'react';
 
-import {deviceNameMapper, loadDeviceListModule} from 'sentry/components/deviceName';
+import {deviceNameMapper} from 'sentry/components/deviceName';
 import TagDistributionMeter from 'sentry/components/tagDistributionMeter';
 import {Group, Organization, TagWithTopValues} from 'sentry/types';
-import {IOSDeviceList} from 'sentry/types/iOSDeviceList';
 
 type Props = {
   group: Group;
@@ -15,26 +14,9 @@ type Props = {
   totalValues: number;
 };
 
-type State = {
-  error: boolean;
-  loading: boolean;
-  iOSDeviceList?: IOSDeviceList;
-};
-
-class GroupTagDistributionMeter extends Component<Props, State> {
-  state: State = {
-    loading: true,
-    error: false,
-  };
-
-  componentDidMount() {
-    this.fetchData();
-  }
-
-  shouldComponentUpdate(nextProps: Props, nextState: State) {
+class GroupTagDistributionMeter extends Component<Props> {
+  shouldComponentUpdate(nextProps: Props) {
     return (
-      this.state.loading !== nextState.loading ||
-      this.state.error !== nextState.error ||
       this.props.tag !== nextProps.tag ||
       this.props.name !== nextProps.name ||
       this.props.totalValues !== nextProps.totalValues ||
@@ -42,40 +24,14 @@ class GroupTagDistributionMeter extends Component<Props, State> {
     );
   }
 
-  fetchData() {
-    this.setState({
-      loading: true,
-      error: false,
-    });
-
-    loadDeviceListModule('iOS')
-      .then(iOSDeviceList => {
-        this.setState({
-          iOSDeviceList,
-          error: false,
-          loading: false,
-        });
-      })
-      .catch(() => {
-        this.setState({
-          error: true,
-          loading: false,
-        });
-      });
-  }
-
   render() {
     const {organization, group, tag, totalValues, topValues} = this.props;
-    const {loading, error, iOSDeviceList} = this.state;
-
     const url = `/organizations/${organization.slug}/issues/${group.id}/tags/${tag}/`;
 
     const segments = topValues
       ? topValues.map(value => ({
           ...value,
-          name: iOSDeviceList
-            ? deviceNameMapper(value.name || '', iOSDeviceList) || ''
-            : value.name,
+          name: deviceNameMapper(value.name || '') || value.name,
           url,
         }))
       : [];
@@ -84,8 +40,8 @@ class GroupTagDistributionMeter extends Component<Props, State> {
       <TagDistributionMeter
         title={tag}
         totalValues={totalValues}
-        isLoading={loading}
-        hasError={error}
+        isLoading={false}
+        hasError={false}
         segments={segments}
       />
     );

+ 155 - 0
static/app/constants/ios-device-list.tsx

@@ -0,0 +1,155 @@
+// THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
+// generated using scripts/extract-ios-device-names.ts as part of build step.
+// the purpose of the script is to extract only the iOS information that Sentry cares about
+// and discard the rest of the JSON so we do not end up bloating bundle size.
+
+const iOSDeviceMapping: Record<string, string> = {
+  'iPod1,1': 'iPod touch',
+  'iPod2,1': 'iPod touch (2nd generation)',
+  'iPod3,1': 'iPod touch (3rd generation)',
+  'iPod4,1': 'iPod touch (4th generation)',
+  'iPod5,1': 'iPod touch (5th generation)',
+  'iPod7,1': 'iPod touch (6th generation)',
+  'iPod9,1': 'iPod touch (7th generation)',
+  'iPhone1,1': 'iPhone',
+  'iPhone1,2': 'iPhone 3G',
+  'iPhone2,1': 'iPhone 3GS',
+  'iPhone3,1': 'iPhone 4',
+  'iPhone3,2': 'iPhone 4',
+  'iPhone3,3': 'iPhone 4',
+  'iPhone4,1': 'iPhone 4S',
+  'iPhone5,1': 'iPhone 5',
+  'iPhone5,2': 'iPhone 5',
+  'iPhone5,3': 'iPhone 5c',
+  'iPhone5,4': 'iPhone 5c',
+  'iPhone6,1': 'iPhone 5s',
+  'iPhone6,2': 'iPhone 5s',
+  'iPhone8,4': 'iPhone SE (1st generation)',
+  'iPhone7,2': 'iPhone 6',
+  'iPhone7,1': 'iPhone 6 Plus',
+  'iPhone8,1': 'iPhone 6s',
+  'iPhone8,2': 'iPhone 6s Plus',
+  'iPhone9,1': 'iPhone 7',
+  'iPhone9,3': 'iPhone 7',
+  'iPhone9,2': 'iPhone 7 Plus',
+  'iPhone9,4': 'iPhone 7 Plus',
+  'iPhone10,1': 'iPhone 8',
+  'iPhone10,4': 'iPhone 8',
+  'iPhone10,2': 'iPhone 8 Plus',
+  'iPhone10,5': 'iPhone 8 Plus',
+  'iPhone10,3': 'iPhone X',
+  'iPhone10,6': 'iPhone X',
+  'iPhone11,8': 'iPhone XR',
+  'iPhone11,2': 'iPhone XS',
+  'iPhone11,4': 'iPhone XS Max',
+  'iPhone11,6': 'iPhone XS Max',
+  'iPhone12,1': 'iPhone 11',
+  'iPhone12,3': 'iPhone 11 Pro',
+  'iPhone12,5': 'iPhone 11 Pro Max',
+  'iPhone12,8': 'iPhone SE (2nd generation)',
+  'iPhone13,1': 'iPhone 12 mini',
+  'iPhone13,2': 'iPhone 12',
+  'iPhone13,3': 'iPhone 12 Pro',
+  'iPhone13,4': 'iPhone 12 Pro Max',
+  'iPad6,7': 'iPad Pro (12.9-inch)',
+  'iPad6,8': 'iPad Pro (12.9-inch)',
+  'iPad6,3': 'iPad Pro (9.7-inch)',
+  'iPad6,4': 'iPad Pro (9.7-inch)',
+  'iPad7,1': 'iPad Pro (12.9-inch, 2nd generation)',
+  'iPad7,2': 'iPad Pro (12.9-inch, 2nd generation)',
+  'iPad7,3': 'iPad Pro (10.5-inch)',
+  'iPad7,4': 'iPad Pro (10.5-inch)',
+  'iPad8,1': 'iPad Pro (11-inch)',
+  'iPad8,2': 'iPad Pro (11-inch)',
+  'iPad8,3': 'iPad Pro (11-inch)',
+  'iPad8,4': 'iPad Pro (11-inch)',
+  'iPad8,5': 'iPad Pro (12.9-inch) (3rd generation)',
+  'iPad8,6': 'iPad Pro (12.9-inch) (3rd generation)',
+  'iPad8,7': 'iPad Pro (12.9-inch) (3rd generation)',
+  'iPad8,8': 'iPad Pro (12.9-inch) (3rd generation)',
+  'iPad8,9': 'iPad Pro (11-inch) (2nd generation)',
+  'iPad8,10': 'iPad Pro (11-inch) (2nd generation)',
+  'iPad8,11': 'iPad Pro (12.9-inch) (4th generation)',
+  'iPad8,12': 'iPad Pro (12.9-inch) (4th generation)',
+  'iPad2,5': 'iPad mini',
+  'iPad2,6': 'iPad mini',
+  'iPad2,7': 'iPad mini',
+  'iPad4,4': 'iPad mini 2',
+  'iPad4,5': 'iPad mini 2',
+  'iPad4,6': 'iPad mini 2',
+  'iPad4,7': 'iPad mini 3',
+  'iPad4,8': 'iPad mini 3',
+  'iPad4,9': 'iPad mini 3',
+  'iPad5,1': 'iPad mini 4',
+  'iPad5,2': 'iPad mini 4',
+  'iPad11,1': 'iPad mini (5th generation)',
+  'iPad11,2': 'iPad mini (5th generation)',
+  'iPad4,1': 'iPad Air',
+  'iPad4,2': 'iPad Air',
+  'iPad4,3': 'iPad Air',
+  'iPad5,3': 'iPad Air 2',
+  'iPad5,4': 'iPad Air 2',
+  'iPad11,3': 'iPad Air (3rd generation)',
+  'iPad11,4': 'iPad Air (3rd generation)',
+  'iPad13,1': 'iPad Air (4th generation)',
+  'iPad13,2': 'iPad Air (4th generation)',
+  'iPad1,1': 'iPad',
+  'iPad2,1': 'iPad 2',
+  'iPad2,2': 'iPad 2',
+  'iPad2,3': 'iPad 2',
+  'iPad2,4': 'iPad 2',
+  'iPad3,1': 'iPad (3rd generation)',
+  'iPad3,2': 'iPad (3rd generation)',
+  'iPad3,3': 'iPad (3rd generation)',
+  'iPad3,4': 'iPad (4th generation)',
+  'iPad3,5': 'iPad (4th generation)',
+  'iPad3,6': 'iPad (4th generation)',
+  'iPad6,11': 'iPad (5th generation)',
+  'iPad6,12': 'iPad (5th generation)',
+  'iPad7,5': 'iPad (6th generation)',
+  'iPad7,6': 'iPad (6th generation)',
+  'iPad7,11': 'iPad (7th generation)',
+  'iPad7,12': 'iPad (7th generation)',
+  'iPad11,6': 'iPad (8th generation)',
+  'iPad11,7': 'iPad (8th generation)',
+  'AudioAccessory1,1': 'HomePod',
+  'AudioAccessory1,2': 'HomePod',
+  'AudioAccessory5,1': 'HomePod mini',
+  'Watch1,1': 'Apple Watch (1st generation)',
+  'Watch1,2': 'Apple Watch (1st generation)',
+  'Watch2,6': 'Apple Watch Series 1',
+  'Watch2,7': 'Apple Watch Series 1',
+  'Watch2,3': 'Apple Watch Series 2',
+  'Watch2,4': 'Apple Watch Series 2',
+  'Watch3,1': 'Apple Watch Series 3',
+  'Watch3,2': 'Apple Watch Series 3',
+  'Watch3,3': 'Apple Watch Series 3',
+  'Watch3,4': 'Apple Watch Series 3',
+  'Watch4,1': 'Apple Watch Series 4',
+  'Watch4,2': 'Apple Watch Series 4',
+  'Watch4,3': 'Apple Watch Series 4',
+  'Watch4,4': 'Apple Watch Series 4',
+  'Watch5,1': 'Apple Watch Series 5',
+  'Watch5,2': 'Apple Watch Series 5',
+  'Watch5,3': 'Apple Watch Series 5',
+  'Watch5,4': 'Apple Watch Series 5',
+  'Watch5,9': 'Apple Watch SE',
+  'Watch5,10': 'Apple Watch SE',
+  'Watch5,11': 'Apple Watch SE',
+  'Watch5,12': 'Apple Watch SE',
+  'Watch6,1': 'Apple Watch Series 6',
+  'Watch6,2': 'Apple Watch Series 6',
+  'Watch6,3': 'Apple Watch Series 6',
+  'Watch6,4': 'Apple Watch Series 6',
+  'AppleTV1,1': 'Apple TV (1st generation)',
+  'AppleTV2,1': 'Apple TV (2nd generation)',
+  'AppleTV3,1': 'Apple TV (3rd generation)',
+  'AppleTV3,2': 'Apple TV (3rd generation)',
+  'AppleTV5,3': 'Apple TV (4th generation)',
+  'AppleTV6,2': 'Apple TV 4K',
+  'AirPods1,1': 'AirPods (1st generation)',
+  'AirPods2,1': 'AirPods (2nd generation)',
+  'iProd8,1': 'AirPods Pro',
+};
+
+export {iOSDeviceMapping};

+ 0 - 41
static/app/types/iOSDeviceList.tsx

@@ -1,41 +0,0 @@
-// https://github.com/pbakondy/ios-device-list#readme
-
-type Options = {
-  caseInsensitive?: boolean;
-  contains?: boolean;
-};
-
-interface Device {
-  ANumber: string | string[];
-  Color: string;
-  FCCID: string | string[];
-  Generation: string;
-  Identifier: string;
-  InternalName: string;
-  Model: string | string[];
-  Storage: string;
-  Type: string;
-  Bootrom?: string | string[];
-  Variant?: string;
-}
-
-export interface IOSDeviceList {
-  anumbers: (type?: string) => [];
-  colors: (type?: string) => [];
-  deviceByColor: (color: string, type?: string, options?: Options) => Device[];
-  deviceByFCCID: (fccid: string[], type?: string, options?: Options) => Device[];
-  deviceByGeneration: (generation: string, type?: string, options?: Options) => Device[];
-  deviceByIdentifier: (id: string, type?: string, options?: Options) => Device[];
-  deviceByInternalName: (name: string, type?: string, options?: Options) => Device[];
-  deviceByModel: (model: string, type?: string, options?: Options) => Device[];
-  deviceByStorage: (storage: string, type?: string, options?: Options) => Device[];
-  deviceTypes: () => string[];
-  devices: (type?: string) => Device[];
-  fccids: (type?: string) => [];
-  generationByIdentifier: (id: string, type?: string) => string | undefined;
-  generations: (type?: string) => [];
-  identifiers: (type?: string) => [];
-  internalNames: (type?: string) => [];
-  models: (type?: string) => [];
-  storages: (type?: string) => [];
-}

+ 5 - 31
tests/js/spec/components/deviceName.spec.tsx

@@ -1,44 +1,18 @@
-import * as Sentry from '@sentry/react';
-import {generationByIdentifier} from 'ios-device-list';
+import {render, screen} from 'sentry-test/reactTestingLibrary';
 
-import {render, screen, waitFor} from 'sentry-test/reactTestingLibrary';
-
-import * as Device from 'sentry/components/deviceName';
+import {DeviceName} from 'sentry/components/deviceName';
 
 jest.mock('ios-device-list');
 
 describe('DeviceName', () => {
   it('renders device name if module is loaded', async () => {
-    generationByIdentifier.mockImplementation(() => 'iPhone 6s Plus');
-
-    render(<Device.DeviceName value="iPhone8,2" />);
-
+    render(<DeviceName value="iPhone8,2" />);
     expect(await screen.findByText('iPhone 6s Plus')).toBeInTheDocument();
-    expect(generationByIdentifier).toHaveBeenCalledWith('iPhone8,2');
   });
 
   it('renders device name if name helper returns undefined', async () => {
-    generationByIdentifier.mockImplementation(() => undefined);
-
-    render(<Device.DeviceName value="iPhone8,2" />);
-
-    expect(await screen.findByText('iPhone8,2')).toBeInTheDocument();
-    expect(generationByIdentifier).toHaveBeenCalledWith('iPhone8,2');
-  });
-
-  it('renders device name if module fails to load', async () => {
-    jest.spyOn(Device, 'loadDeviceListModule').mockImplementation(() => {
-      return Promise.reject('Cannot load module');
-    });
-
-    const spy = jest.spyOn(Sentry, 'captureException');
-
-    render(<Device.DeviceName value="iPhone8,2" />);
-
-    await waitFor(() =>
-      expect(spy).toHaveBeenCalledWith('Failed to load ios-device-list module')
-    );
+    render(<DeviceName value="iPhone8,2FunkyValue" />);
 
-    expect(await screen.findByText('iPhone8,2')).toBeInTheDocument();
+    expect(await screen.findByText('iPhone8,2FunkyValue')).toBeInTheDocument();
   });
 });

+ 5 - 1
tests/js/spec/components/events/eventTagsAndScreenshot.spec.tsx

@@ -4,6 +4,8 @@ import {render, screen, within} from 'sentry-test/reactTestingLibrary';
 import EventTagsAndScreenshot from 'sentry/components/events/eventTagsAndScreenshot';
 import {EventAttachment} from 'sentry/types';
 
+import {deviceNameMapper} from '../../../../../static/app/components/deviceName';
+
 describe('EventTagsAndScreenshot ', function () {
   const contexts = {
     app: {
@@ -180,7 +182,9 @@ describe('EventTagsAndScreenshot ', function () {
 
       // Context Item 3
       const contextItem3 = within(contextItems[2]);
-      expect(contextItem3.getByRole('heading')).toHaveTextContent(contexts.device.model);
+      expect(contextItem3.getByRole('heading')).toHaveTextContent(
+        deviceNameMapper(contexts.device.model)?.trim() ?? ''
+      );
       expect(contextItem3.getByTestId('context-sub-title')).toHaveTextContent(
         `Model:${contexts.device.model_id}`
       );

Some files were not shown because too many files changed in this diff