-
Notifications
You must be signed in to change notification settings - Fork 6
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
Use non-forked rrweb #34
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import { test, expect } from '../src'; | ||
|
||
test('asset paths', async ({ page }) => { | ||
await page.goto('/asset-paths'); | ||
await page.goto('/asset-paths/'); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,7 +48,6 @@ | |
"@babel/preset-react": "^7.18.6", | ||
"@babel/preset-typescript": "^7.21.4", | ||
"@chromaui/archive-storybook": "^0.0.15", | ||
"@chromaui/test-archiver": "^0.0.32", | ||
"@jest/types": "^27.0.6", | ||
"@playwright/test": "^1.39.0", | ||
"@storybook/addon-essentials": "^7.5.2", | ||
|
@@ -97,10 +96,10 @@ | |
"access": "public" | ||
}, | ||
"dependencies": { | ||
"@chromaui/rrweb-snapshot": "2.0.0-alpha.7-noAbsolute.2", | ||
"@segment/analytics-node": "^1.1.0", | ||
"fs-extra": "^11.1.1", | ||
"mime": "^3.0.0", | ||
"rrweb-snapshot": "^2.0.0-alpha.4", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rrweb is up to like |
||
"ts-dedent": "^2.2.0" | ||
}, | ||
"peerDependencies": { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,8 +30,7 @@ export class ArchiveFile { | |
} | ||
|
||
originalSrc() { | ||
// Assets that we capture will be stripped of the domain in the source | ||
return `${this.url.pathname}${this.url.search}`; | ||
return this.url.toString(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the main change required after all of the previous asset mapping work. We're now expecting all asset paths to be full URLs since that's what rrweb does, so we don't strip off the protocol and origin here anymore. |
||
} | ||
|
||
toFileSystemPath() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
import fs from 'fs-extra'; | ||
import { resolve } from 'path'; | ||
import type { TestInfo } from '@playwright/test'; | ||
import { NodeType } from '@chromaui/rrweb-snapshot'; | ||
import { NodeType } from 'rrweb-snapshot'; | ||
import { writeTestResult } from '.'; | ||
|
||
jest.mock('fs-extra'); | ||
|
@@ -11,13 +11,13 @@ const snapshotJson = { | |
{ | ||
type: NodeType.Element, | ||
attributes: { | ||
src: '/home/', | ||
src: 'http://localhost:3000/home/', | ||
}, | ||
}, | ||
{ | ||
type: NodeType.Element, | ||
attributes: { | ||
src: '/img?src=some-path', | ||
src: 'http://localhost:3000/img?src=some-path', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we save the URLs like this in the archives or as relative paths? If we save as absolutes, that would be a non-starter. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The asset archive is built by watching network requests, so they are always the full URL there. This change makes it easier to match the assets from the archive with the attributes in the DOM since both sides will be full URLs now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me rephrase my question. What happens when you render in the Capture Cloud? Do the archives try to pull from a relative path or http://localhost:3000? The localhost lookup will not work in the capture cloud so those need to be replaced by relative paths There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yeah, they are relative in the archive storybook so that they load from the capture cloud. Otherwise all the images in the build would be broken! |
||
}, | ||
}, | ||
], | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2052,17 +2052,6 @@ | |
resolved "https://registry.npmjs.org/@chromaui/rrweb-snapshot/-/rrweb-snapshot-2.0.0-alpha.7-noAbsolute.2.tgz" | ||
integrity sha512-vOVSOgfNyxomxm6ENMqm1C4rLkLLOlyEyhR5J2SKBALuFfuJHjhIqNLSOgqVRO9J3++AUOCktknyJ/F+mSvEfA== | ||
|
||
"@chromaui/test-archiver@^0.0.32": | ||
version "0.0.32" | ||
resolved "https://registry.yarnpkg.com/@chromaui/test-archiver/-/test-archiver-0.0.32.tgz#6e0560b39108581b9d3d55d3f5e462350b3cda9c" | ||
integrity sha512-O6le42KqoliioAQG2RY+XhrwSttwnYHV2lIk3TNKJ1l9sTjQT5AUAZJ6N4CkUAsU8TVtX8wEDUY0mrNL00M/DQ== | ||
dependencies: | ||
"@chromaui/rrweb-snapshot" "2.0.0-alpha.7-noAbsolute.2" | ||
"@segment/analytics-node" "^1.1.0" | ||
fs-extra "^11.1.1" | ||
mime "^3.0.0" | ||
ts-dedent "^2.2.0" | ||
|
||
"@colors/[email protected]": | ||
version "1.5.0" | ||
resolved "https://registry.yarnpkg.com/@colors/colors/-/colors-1.5.0.tgz#bb504579c1cae923e6576a4f5da43d25f97bdbd9" | ||
|
@@ -11571,6 +11560,11 @@ rollup@^3.2.5: | |
optionalDependencies: | ||
fsevents "~2.3.2" | ||
|
||
rrweb-snapshot@^2.0.0-alpha.4: | ||
version "2.0.0-alpha.4" | ||
resolved "https://registry.yarnpkg.com/rrweb-snapshot/-/rrweb-snapshot-2.0.0-alpha.4.tgz#2801bf5946177b9d685a01661a62d9d2e958f174" | ||
integrity sha512-KQ2OtPpXO5jLYqg1OnXS/Hf+EzqnZyP5A+XPqBCjYpj3XIje/Od4gdUwjbFo3cVuWq5Cw5Y1d3/xwgIS7/XpQQ== | ||
|
||
run-parallel@^1.1.9: | ||
version "1.2.0" | ||
resolved "https://registry.npmjs.org/run-parallel/-/run-parallel-1.2.0.tgz" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thafryer added that relative path test for AP-3789