Skip to content

Commit

Permalink
fix: remove jsoncrush
Browse files Browse the repository at this point in the history
  • Loading branch information
k11kirky committed Dec 19, 2024
1 parent 0a63875 commit 4b71fed
Show file tree
Hide file tree
Showing 7 changed files with 12 additions and 38 deletions.
5 changes: 1 addition & 4 deletions cypress/e2e/insights-reload-query.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import JSONCrush from 'jsoncrush'

describe('ReloadInsight component', () => {
beforeEach(() => {
// Clear local storage before each test to ensure a clean state
Expand All @@ -21,8 +19,7 @@ describe('ReloadInsight component', () => {
const draftQuery = window.localStorage.getItem(`draft-query-${currentTeamId}`)
expect(draftQuery).to.not.be.null

const draftQueryObjUncrushed = JSONCrush.uncrush(draftQuery)
const draftQueryObj = JSON.parse(draftQueryObjUncrushed)
const draftQueryObj = JSON.parse(draftQuery)

expect(draftQueryObj).to.have.property('query')

Expand Down
1 change: 1 addition & 0 deletions frontend/src/scenes/insights/insightDataLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ export const insightDataLogic = kea<insightDataLogicType>([
if (isQueryTooLarge(query)) {
localStorage.removeItem(`draft-query-${values.currentTeamId}`)
}

localStorage.setItem(
`draft-query-${values.currentTeamId}`,
crushDraftQueryForLocalStorage(query, Date.now())
Expand Down
30 changes: 8 additions & 22 deletions frontend/src/scenes/insights/utils.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import JSONCrush from 'jsoncrush'
import api from 'lib/api'
import { dayjs } from 'lib/dayjs'
import { CORE_FILTER_DEFINITIONS_BY_GROUP } from 'lib/taxonomy'
Expand Down Expand Up @@ -445,40 +444,27 @@ export function isQueryTooLarge(query: Node<Record<string, any>>): boolean {
export function parseDraftQueryFromLocalStorage(
query: string
): { query: Node<Record<string, any>>; timestamp: number } | null {
// First try to uncrush the query if it's a JSONCrush query else fall back to parsing it as a JSON
try {
const uncrushedQuery = JSONCrush.uncrush(query)
return JSON.parse(uncrushedQuery)
return JSON.parse(query)
} catch (e) {
console.error('Error parsing uncrushed query', e)
try {
return JSON.parse(query)
} catch (e) {
console.error('Error parsing query', e)
return null
}
console.error('Error parsing query', e)
return null
}
}

export function crushDraftQueryForLocalStorage(query: Node<Record<string, any>>, timestamp: number): string {
return JSONCrush.crush(JSON.stringify({ query, timestamp }))
return JSON.stringify({ query, timestamp })
}

export function parseDraftQueryFromURL(query: string): Node<Record<string, any>> | null {
try {
const uncrushedQuery = JSONCrush.uncrush(query)
return JSON.parse(uncrushedQuery)
return JSON.parse(query)
} catch (e) {
console.error('Error parsing uncrushed query', e)
try {
return JSON.parse(query)
} catch (e) {
console.error('Error parsing query', e)
return null
}
console.error('Error parsing query', e)
return null
}
}

export function crushDraftQueryForURL(query: Node<Record<string, any>>): string {
return JSONCrush.crush(JSON.stringify(query))
return JSON.stringify(query)
}
4 changes: 1 addition & 3 deletions frontend/src/scenes/urls.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import JSONCrush from 'jsoncrush'
import { combineUrl } from 'kea-router'
import { AlertType } from 'lib/components/Alerts/types'
import { getCurrentTeamId } from 'lib/utils/getAppContext'
Expand Down Expand Up @@ -75,8 +74,7 @@ export const urls = {
insightNew: (type?: InsightType, dashboardId?: DashboardType['id'] | null, query?: Node): string =>
combineUrl('/insights/new', dashboardId ? { dashboard: dashboardId } : {}, {
...(type ? { insight: type } : {}),
// have to use JSONCrush directly rather than the util to avoid circular dep
...(query ? { q: typeof query === 'string' ? query : JSONCrush.crush(JSON.stringify(query)) } : {}),
...(query ? { q: typeof query === 'string' ? query : JSON.stringify(query) } : {}),
}).url,
insightNewHogQL: (query: string, filters?: HogQLFilters): string =>
combineUrl(
Expand Down
2 changes: 1 addition & 1 deletion jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ process.env.TZ = process.env.TZ || 'UTC'
* https://jestjs.io/docs/en/configuration.html
*/

const esmModules = ['query-selector-shadow-dom', 'react-syntax-highlighter', '@react-hook', '@medv', 'monaco-editor', 'jsoncrush']
const esmModules = ['query-selector-shadow-dom', 'react-syntax-highlighter', '@react-hook', '@medv', 'monaco-editor']
const eeFolderExists = fs.existsSync('ee/frontend/exports.ts')
function rootDirectories() {
const rootDirectories = ['<rootDir>/frontend/src']
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@
"hls.js": "^1.5.15",
"husky": "^7.0.4",
"image-blob-reduce": "^4.1.0",
"jsoncrush": "^1.1.8",
"kea": "^3.1.5",
"kea-forms": "^3.2.0",
"kea-loaders": "^3.0.0",
Expand Down
7 changes: 0 additions & 7 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 4b71fed

Please sign in to comment.