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

feat(core): replace uuid dependency with crypto.randomUUID() #976

Merged
merged 6 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions editor/Editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import EditorPanel, { type EditorLangauge } from './EditorPanel';
import EditorExamples from './EditorExamples';

import './Editor.css';
import { v4 } from 'uuid';
import { uuid } from '../src/core/utils/uuid';

function json2js(jsonCode: string) {
return `var spec = ${jsonCode} \nexport { spec }; \n`;
Expand Down Expand Up @@ -723,7 +723,7 @@ function Editor(props: RouteComponentProps) {
}
return (
<div
key={v4()}
key={uuid()}
Copy link
Member Author

Choose a reason for hiding this comment

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

Just a side observation. I'm guessing the uuid is used here to please eslint-react, but I don't think key-ing with a UUID is desirable. Ideally it would be some piece of data from the array of objects that is deterministic between renders.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for finding this! This should have used the id of the view or track instead. I will update this after testing it.

style={{
position: 'absolute',
border: '1px solid black',
Expand Down
4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@
"quick-lru": "^6.1.1",
"rbush": "^3.0.1",
"stream-browserify": "^3.0.0",
"threads": "^1.6.4",
"uuid": "^8.3.2"
"threads": "^1.6.4"
},
"devDependencies": {
"@types/bezier-js": "^4.1.0",
Expand All @@ -94,7 +93,6 @@
"@types/react-dom": "^18.2.0",
"@types/react-resize-detector": "^4.2.0",
"@types/react-router-dom": "^5.1.6",
"@types/uuid": "^8.3.1",
"@typescript-eslint/eslint-plugin": "^5.56.0",
"@typescript-eslint/parser": "^5.56.0",
"@vitejs/plugin-react": "^4.1.0",
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/gosling-to-higlass.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as uuid from 'uuid';
import type { Track as HiGlassTrack } from '@gosling-lang/higlass-schema';
import { HiGlassModel, HIGLASS_AXIS_SIZE } from './higlass-model';
import { parseServerAndTilesetUidFromUrl } from '../core/utils';
Expand All @@ -20,7 +19,8 @@ import { DEWFAULT_TITLE_PADDING_ON_TOP_AND_BOTTOM } from './defaults';
import type { CompleteThemeDeep } from '../core/utils/theme';
import { DEFAULT_TEXT_STYLE } from '../core/utils/text-style';
import type { GoslingToHiGlassIdMapper } from '../api/track-and-view-ids';
import type { UrlToFetchOptions } from 'src/core/gosling-component';
import type { UrlToFetchOptions } from '../core/gosling-component';
import { uuid } from '../core/utils/uuid';

/**
* Convert a gosling track into a HiGlass view and add it into a higlass model.
Expand All @@ -47,7 +47,7 @@ export function goslingToHiGlass(
const firstResolvedSpec = resolvedSpecs[0];

// If missing, create a unique track ID that will be used as HiGlass view ID for caching
const trackId = firstResolvedSpec.id ?? uuid.v4();
const trackId = firstResolvedSpec.id ?? uuid();
if (!firstResolvedSpec.id) {
firstResolvedSpec.id = trackId;
}
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/higlass-model.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as uuid from 'uuid';
import { HiGlassModel } from './higlass-model';
import { computeChromSizes } from '../core/utils/assembly';
import { getTheme } from '../core/utils/theme';
import { uuid } from '../core/utils/uuid';

describe('Should produce higlass model correctly', () => {
it('Should set default values correctly', () => {
Expand All @@ -11,7 +11,7 @@ describe('Should produce higlass model correctly', () => {

it('Should set domain correctly', () => {
const higlass = new HiGlassModel();
higlass.addDefaultView(uuid.v1());
higlass.addDefaultView(uuid());
higlass.setDomain({ chromosome: 'chr2' }, { chromosome: 'chr2', interval: [100, 200] });
expect(higlass.spec().views?.[0].initialXDomain).toEqual([
computeChromSizes().interval['chr2'][0] + 1,
Expand All @@ -23,7 +23,7 @@ describe('Should produce higlass model correctly', () => {

it('Should add brush correctly', () => {
const higlass = new HiGlassModel();
higlass.addDefaultView(uuid.v1());
higlass.addDefaultView(uuid());
higlass.addBrush('linear', higlass.getLastView().uid ?? '', getTheme(), 'from');
expect(JSON.stringify(higlass.spec())).toContain('viewport-projection-horizontal');
});
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/higlass-model.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as uuid from 'uuid';
import type { HiGlassSpec, Track } from '@gosling-lang/higlass-schema';
import { HiGlassSchema } from '@gosling-lang/higlass-schema';
import type { Assembly, AxisPosition, Domain, DummyTrack, Orientation, ZoomLimits } from '@gosling-lang/gosling-schema';
Expand All @@ -9,6 +8,7 @@ import { getAutoCompleteId, computeChromSizes } from '../core/utils/assembly';
import type { CompleteThemeDeep } from '../core/utils/theme';
import exampleHg from '../core/example/hg-view-config-1';
import { insertItemToArray } from '../core/utils/array';
import { uuid } from '../core/utils/uuid';

export const HIGLASS_AXIS_SIZE = 30;

Expand Down Expand Up @@ -163,7 +163,7 @@ export class HiGlassModel {
(this.getView(viewId) as any)?.tracks.whole.push({
// type: 'viewport-projection-center',
type: layout === 'circular' ? 'brush-track' : 'viewport-projection-horizontal',
uid: uuid.v4(),
uid: uuid(),
fromViewUid,
options: {
projectionFillColor: style?.color ?? theme.brush.color,
Expand Down Expand Up @@ -288,7 +288,7 @@ export class HiGlassModel {
this.getLastView().tracks[this.getMainTrackPosition()] = [
{
type: 'combined',
uid: `${track.uid ?? uuid.v4()}-${this.getMainTrackPosition()}-combined`,
uid: `${track.uid ?? uuid()}-${this.getMainTrackPosition()}-combined`,
// !! Hacky, but it is important to subtract 1px. Currently, HiGlass does not well handle a case where a center track is zero width (e.g., linking between views that contain zero-width center tracks).
// https://github.com/higlass/higlass/pull/1041
width: (track as any).width - 1,
Expand Down Expand Up @@ -324,7 +324,7 @@ export class HiGlassModel {

const widthOrHeight = position === 'left' || position === 'right' ? 'width' : 'height';
const axisTrackTemplate: Track = {
// uid: options.id ?? uuid.v1(), // TODO: turning this on makes some tick labels hidden
// uid: options.id ?? uuid(), // TODO: turning this on makes some tick labels hidden
type: 'axis-track',
chromInfoPath: this.hg.chromInfoPath,
options: {
Expand Down
10 changes: 5 additions & 5 deletions src/compiler/spec-preprocess.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as uuid from 'uuid';
import type {
SingleTrack,
GoslingSpec,
Expand Down Expand Up @@ -31,6 +30,7 @@ import {
} from './defaults';
import { spreadTracksByData } from '../core/utils/overlay';
import { getStyleOverridden } from '../core/utils/style';
import { uuid } from '../core/utils/uuid';

/**
* Traverse individual tracks and call the callback function to read and/or update the track definition.
Expand Down Expand Up @@ -176,7 +176,7 @@ export function traverseToFixSpecDownstream(spec: GoslingSpec | SingleView, pare

// ID should be assigned to each view and track for an API usage
if (!spec.id) {
spec.id = uuid.v4();
spec.id = uuid();
}

if ('tracks' in spec) {
Expand All @@ -188,11 +188,11 @@ export function traverseToFixSpecDownstream(spec: GoslingSpec | SingleView, pare
*/
tracks = spreadTracksByData(tracks);

const linkID = uuid.v4();
const linkID = uuid();
tracks.forEach((track, i, array) => {
// ID should be assigned to each view and track for an API usage
if (!track.id) {
track.id = uuid.v4();
track.id = uuid();
}

// If size not defined, set default ones
Expand All @@ -215,7 +215,7 @@ export function traverseToFixSpecDownstream(spec: GoslingSpec | SingleView, pare
track.xe.field
// Question: Should we consider mark types? (e.g., link might not be supported?)
) {
const newField = uuid.v4();
const newField = uuid();
const startField = track.x.field;
const endField = track.xe.field;
const padding = track.displacement.padding;
Expand Down
4 changes: 2 additions & 2 deletions src/core/gosling-component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import { createApi, type GoslingApi } from '../api/api';
import { GoslingTemplates } from '..';
import { omitDeep } from './utils/omit-deep';
import { isEqual } from 'lodash-es';
import * as uuid from 'uuid';
import { publish } from '../api/pubsub';
import type { IdTable } from '../api/track-and-view-ids';
import { preverseZoomStatus } from './utils/higlass-zoom-config';
import { uuid } from '../core/utils/uuid';

// Before rerendering, wait for a few time so that HiGlass container is resized already.
// If HiGlass is rendered and then the container resizes, the viewport position changes, unmatching `xDomain` specified by users.
Expand Down Expand Up @@ -59,7 +59,7 @@ export const GoslingComponent = forwardRef<GoslingRef, GoslingCompProps>((props,
const hgRef = useRef<HiGlassApi>(null);

const theme = getTheme(props.theme || 'light');
const wrapperDivId = props.id ?? uuid.v4();
const wrapperDivId = props.id ?? uuid();

/**
* Publishes event if there is a new view added
Expand Down
6 changes: 3 additions & 3 deletions src/core/higlass-component-wrapper.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/* eslint-disable react/prop-types */
import type * as PIXI from 'pixi.js';
import React, { useEffect, useState, forwardRef, useMemo } from 'react';
import * as uuid from 'uuid';

import * as gosling from '..';
// @ts-ignore
import { HiGlassComponent } from 'higlass';
import type { HiGlassSpec } from '@gosling-lang/higlass-schema';
import { uuid } from '../core/utils/uuid';

/**
* Register plugin tracks and data fetchers to HiGlass. This is necessary for the first time before using Gosling.
Expand Down Expand Up @@ -39,9 +39,9 @@ export interface HiGlassComponentWrapperProps {
export const HiGlassComponentWrapper = forwardRef<HiGlassApi | undefined, HiGlassComponentWrapperProps>(
(props, ref) => {
// div `id` and `className` for detailed customization
const [wrapperDivId, setWrapperDivId] = useState(props.id ?? uuid.v4());
const [wrapperDivId, setWrapperDivId] = useState(props.id ?? uuid());
useEffect(() => {
setWrapperDivId(props.id ?? uuid.v4());
setWrapperDivId(props.id ?? uuid());
}, [props.id]);

const viewConfig = props.viewConfig || {};
Expand Down
8 changes: 8 additions & 0 deletions src/core/utils/uuid.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// for envs where crypto.randomUUID is not available (i.e., Node).
function fallback(): string {
return Math.random().toString(36).substring(2, 10);
}

export function uuid(): string {
return globalThis.crypto.randomUUID?.() ?? fallback();
}
4 changes: 2 additions & 2 deletions src/tracks/gosling-brush/brush-track.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { arc as d3arc } from 'd3-shape';
import type { SubjectPosition, D3DragEvent } from 'd3-drag';
import * as uuid from 'uuid';
import { RADIAN_GAP, valueToRadian } from '../../core/utils/polar';
import { uuid } from '../../core/utils/uuid';

type CircularBrushData = {
type: 'brush' | 'start' | 'end';
Expand All @@ -25,7 +25,7 @@ function BrushTrack(HGC: any, ...args: any[]): any {
const [context, options] = params;
const { registerViewportChanged, removeViewportChanged, setDomainsCallback } = context;

this.uid = uuid.v1();
this.uid = uuid();
this.options = options;

// Is there actually a linked from view? Or is this projection "independent"?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
isPointNearPoint,
isCircleWithinRange
} from './polygon';
import * as uuid from 'uuid';
import { uuid } from '../../../core/utils/uuid';

export type MouseEventData = PointEventData | LineEventData | PolygonEventData;

Expand Down Expand Up @@ -52,21 +52,21 @@ export class MouseEventModel {
* Add a new mouse event that is polygon-based.
*/
public addPolygonBasedEvent(value: Datum, polygon: number[]) {
this.data.push({ uid: uuid.v4(), type: 'polygon', value, polygon });
this.data.push({ uid: uuid(), type: 'polygon', value, polygon });
}

/**
* Add a new mouse event that is point-based.
*/
public addPointBasedEvent(value: Datum, pointAndRadius: [number, number, number]) {
this.data.push({ uid: uuid.v4(), type: 'point', value, polygon: pointAndRadius });
this.data.push({ uid: uuid(), type: 'point', value, polygon: pointAndRadius });
}

/**
* Add a new mouse event that is line-based.
*/
public addLineBasedEvent(value: Datum, path: number[]) {
this.data.push({ uid: uuid.v4(), type: 'line', value, polygon: path });
this.data.push({ uid: uuid(), type: 'line', value, polygon: path });
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/tracks/gosling-track/gosling-track-model.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as uuid from 'uuid';
import type {
ChannelDeep,
PredefinedColors,
Expand Down Expand Up @@ -42,6 +41,7 @@ import {
} from '@gosling-lang/gosling-schema';
import { CHANNEL_DEFAULTS } from '../../core/channel';
import { type CompleteThemeDeep, getTheme } from '../../core/utils/theme';
import { uuid } from '../../core/utils/uuid';
import { MouseEventModel } from '../gosling-track/gosling-mouse-event';

export type ScaleType =
Expand Down Expand Up @@ -73,7 +73,7 @@ export class GoslingTrackModel {
private mouseEventModel: MouseEventModel;

constructor(spec: SingleTrack, data: { [k: string]: number | string }[], theme: Required<CompleteThemeDeep>) {
this.id = uuid.v1();
this.id = uuid();

this.theme = theme ?? getTheme();

Expand Down
10 changes: 5 additions & 5 deletions src/tracks/gosling-track/gosling-track.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type * as PIXI from 'pixi.js';
import * as uuid from 'uuid';
import { isEqual, sampleSize, uniqBy } from 'lodash-es';
import type { ScaleLinear } from 'd3-scale';
import type {
Expand Down Expand Up @@ -52,6 +51,7 @@ import {
import { HIGLASS_AXIS_SIZE } from '../../compiler/higlass-model';
import { flatArrayToPairArray } from '../../core/utils/array';
import { createPluginTrack, type PluginTrackFactory, type TrackConfig } from '../../core/utils/define-plugin-track';
import { uuid } from '../../core/utils/uuid';

// Set `true` to print in what order each function is called
export const PRINT_RENDERING_CYCLE = false;
Expand Down Expand Up @@ -188,12 +188,12 @@ const factory: PluginTrackFactory<Tile, GoslingTrackOptions> = (HGC, context, op
this.#assembly = this.options.spec.assembly;

// Add unique IDs to each of the overlaid tracks that will be rendered independently.
if ('_overlay' in this.options.spec) {
this.options.spec._overlay = (this.options.spec as OverlaidTrack)._overlay.map(o => {
return { ...o, _renderingId: uuid.v1() };
if ('overlay' in this.options.spec) {
this.options.spec.overlay = (this.options.spec as OverlaidTrack)._overlay.map(o => {
return { ...o, _renderingId: uuid() };
});
} else {
this.options.spec._renderingId = uuid.v1();
this.options.spec._renderingId = uuid();
}

this.fetchedTiles = {};
Expand Down
3 changes: 1 addition & 2 deletions vite.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,10 @@ const alias = {
'@gosling-lang/dummy-track': path.resolve(__dirname, './src/tracks/dummy-track/index.ts'),
'@data-fetchers': path.resolve(__dirname, './src/data-fetchers/index.ts'),
zlib: path.resolve(__dirname, './src/alias/zlib.ts'),
uuid: path.resolve(__dirname, './node_modules/uuid/dist/esm-browser/index.js'),
stream: path.resolve(__dirname, './node_modules/stream-browserify') // gmod/gff uses stream-browserify
};

const skipExt = new Set(['@gmod/bbi', 'uuid']);
const skipExt = new Set(['@gmod/bbi']);
const external = [...Object.keys(pkg.dependencies), ...Object.keys(pkg.peerDependencies)].filter(
dep => !skipExt.has(dep)
);
Expand Down
5 changes: 0 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1278,11 +1278,6 @@
resolved "https://registry.yarnpkg.com/@types/unist/-/unist-2.0.6.tgz#250a7b16c3b91f672a24552ec64678eeb1d3a08d"
integrity sha512-PBjIUxZHOuj0R15/xuwJYjFi+KZdNFrehocChv4g5hu6aFroHue8m0lBP0POdK2nKzbw0cgV1mws8+V/JAcEkQ==

"@types/uuid@^8.3.1":
version "8.3.4"
resolved "https://registry.yarnpkg.com/@types/uuid/-/uuid-8.3.4.tgz#bd86a43617df0594787d38b735f55c805becf1bc"
integrity sha512-c/I8ZRb51j+pYGAu5CrFMRxqZ2ke4y2grEBO5AUjgSkSk+qT2Ea+OdWElz/OiMf5MNpn2b17kuVBwZLQJXzihw==

"@typescript-eslint/eslint-plugin@^5.56.0":
version "5.58.0"
resolved "https://registry.yarnpkg.com/@typescript-eslint/eslint-plugin/-/eslint-plugin-5.58.0.tgz#b1d4b0ad20243269d020ef9bbb036a40b0849829"
Expand Down