Skip to content

Commit ffa36a2

Browse files
committed
Addressing copilot comment
Copilot spotted that ?browserPreview=true made the cache locale bypass apply too broadly. This change scopes that bypass to embedded browser previews by passing the existing embedded prop into useProject. Non-embedded routes with ?browserPreview=true now still require the cached locale to match, while embedded preview tabs can still load unsaved localized changes. Added tests for both cases.
1 parent cf42ffa commit ffa36a2

4 files changed

Lines changed: 59 additions & 2 deletions

File tree

src/containers/WebComponentLoader.jsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ const WebComponentLoader = (props) => {
160160
remixLoadFailed,
161161
initialProject,
162162
locale,
163+
embedded,
163164
});
164165

165166
useProjectPersistence({

src/containers/WebComponentLoader.test.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,9 +262,28 @@ describe("When no user is in state", () => {
262262
loadCache: true,
263263
remixLoadFailed: false,
264264
reactAppApiEndpoint: "http://localhost:3009",
265+
embedded: false,
265266
});
266267
});
267268

269+
test("Passes embedded prop to useProject hook", () => {
270+
render(
271+
<Provider store={store}>
272+
<CookiesProvider cookies={cookies}>
273+
<WebComponentLoader
274+
code={code}
275+
identifier={identifier}
276+
embedded={true}
277+
/>
278+
</CookiesProvider>
279+
</Provider>,
280+
);
281+
282+
expect(useProject).toHaveBeenLastCalledWith(
283+
expect.objectContaining({ embedded: true }),
284+
);
285+
});
286+
268287
test("Calls useProjectPersistence hook with correct attributes", () => {
269288
expect(useProjectPersistence).toHaveBeenCalledWith({
270289
project: {
@@ -409,6 +428,7 @@ describe("When no user is in state", () => {
409428
loadCache: true,
410429
remixLoadFailed: false,
411430
reactAppApiEndpoint: "http://localhost:3009",
431+
embedded: false,
412432
});
413433
});
414434
});
@@ -536,6 +556,7 @@ describe("When user is in state", () => {
536556
loadCache: true,
537557
remixLoadFailed: false,
538558
reactAppApiEndpoint: "http://localhost:3009",
559+
embedded: false,
539560
});
540561
});
541562

@@ -568,6 +589,7 @@ describe("When user is in state", () => {
568589
loadCache: true,
569590
remixLoadFailed: false,
570591
reactAppApiEndpoint: "http://localhost:3009",
592+
embedded: false,
571593
});
572594
});
573595
});
@@ -650,6 +672,7 @@ describe("When user is in state", () => {
650672
loadCache: true,
651673
remixLoadFailed: true,
652674
reactAppApiEndpoint: "http://localhost:3009",
675+
embedded: false,
653676
});
654677
});
655678

@@ -813,6 +836,7 @@ describe("when a Scratch remix updates the project identifier", () => {
813836
loadCache: true,
814837
remixLoadFailed: false,
815838
reactAppApiEndpoint: "http://localhost:3009",
839+
embedded: false,
816840
});
817841
});
818842
});

src/hooks/useProject.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export const useProject = ({
1616
loadCache = true,
1717
remixLoadFailed = false,
1818
locale = null,
19+
embedded = false,
1920
}) => {
2021
const loading = useSelector((state) => state.editor.loading);
2122
const isEmbedded = useSelector((state) => state.editor.isEmbedded);
@@ -24,7 +25,7 @@ export const useProject = ({
2425
new URLSearchParams(window.location.search).get("browserPreview") ===
2526
"true";
2627
const shouldLoadBrowserPreviewCache =
27-
isBrowserPreview || browserPreviewFromQuery;
28+
isBrowserPreview || (embedded && browserPreviewFromQuery);
2829
const project = useSelector((state) => state.editor.project);
2930
const loadDispatched = useRef(false);
3031

src/hooks/useProject.test.js

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,31 @@ describe("When not embedded", () => {
140140
);
141141
});
142142

143+
test("If cached project does not match locale and browserPreview query is used outside embedded viewer, does not use cached project", () => {
144+
syncProject.mockImplementation(jest.fn((_) => jest.fn()));
145+
window.history.pushState(
146+
{},
147+
"",
148+
"/en-US/projects/hello-world-project?browserPreview=true&page=index.html",
149+
);
150+
localStorage.setItem(
151+
cachedProject.identifier,
152+
JSON.stringify(cachedProject),
153+
);
154+
renderHook(
155+
() =>
156+
useProject({
157+
projectIdentifier: cachedProject.identifier,
158+
locale: "en-US",
159+
accessToken,
160+
reactAppApiEndpoint,
161+
}),
162+
{ wrapper },
163+
);
164+
expect(syncProject).toHaveBeenCalledWith("load");
165+
expect(setProject).not.toHaveBeenCalledWith(cachedProject);
166+
});
167+
143168
test("If cached project does not match identifier and locale, loads correct uncached project", async () => {
144169
syncProject.mockImplementationOnce(jest.fn((_) => loadProject));
145170
localStorage.setItem("project", JSON.stringify(cachedProject));
@@ -559,6 +584,7 @@ describe("When not embedded", () => {
559584

560585
afterEach(() => {
561586
localStorage.clear();
587+
window.history.pushState({}, "", "/");
562588
});
563589
});
564590

@@ -581,7 +607,11 @@ describe("When embedded", () => {
581607
identifier: "blank-html-starter",
582608
locale: "en",
583609
};
584-
window.history.pushState({}, "", "/?browserPreview=true&page=index.html");
610+
window.history.pushState(
611+
{},
612+
"",
613+
"/en-US/embed/viewer/blank-html-starter?browserPreview=true&page=index.html",
614+
);
585615
localStorage.setItem(
586616
browserPreviewCachedProject.identifier,
587617
JSON.stringify(browserPreviewCachedProject),
@@ -591,6 +621,7 @@ describe("When embedded", () => {
591621
useProject({
592622
projectIdentifier: browserPreviewCachedProject.identifier,
593623
locale: "en-US",
624+
embedded: true,
594625
accessToken,
595626
reactAppApiEndpoint,
596627
}),

0 commit comments

Comments
 (0)