Skip to content

Commit

Permalink
Fix atoms leak (#1312)
Browse files Browse the repository at this point in the history
## Description

Atom data leaked into the server canvas.
ref #1313 

## Steps for reproduction

Open Vercel app. Any project. Reload a few times not fast to avoid
scaling.
Get the iframe link, then open it. Should be empty.

New relic logs must contain:
```
{
    assetsStore: 0,
    pagesStore: 0,
    instancesStore: 0,
}
```

New relic query 
`"proxy.path":"*71f88d3e-9231-489e-8ba0-8adc10215335*"
"message":"*assetsStore*"`
where `71f88d3e-9231-489e-8ba0-8adc10215335` is your project id 
(add vercel host 


## Code Review

- [ ] hi @kof, I need you to do
  - conceptual review (architecture, feature-correctness)
  - detailed review (read every line)
  - test it on preview

## Before requesting a review

- [ ] made a self-review
- [x] added inline comments where things may be not obvious (the "why",
not "what")

## Before merging

- [x] tested locally and on preview environment (preview dev login:
5de6)
- [ ] updated [test
cases](https://github.com/webstudio-is/webstudio-builder/blob/main/apps/builder/docs/test-cases.md)
document
- [ ] added tests
- [ ] if any new env variables are added, added them to `.env.example`
and the `builder/env-check.js` if mandatory
  • Loading branch information
istarkov authored Mar 28, 2023
1 parent daf9328 commit 13db987
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 9 deletions.
9 changes: 9 additions & 0 deletions apps/builder/app/canvas/canvas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
assetsStore,
pagesStore,
useRootInstance,
instancesStore,
useSubscribeScrollState,
useIsPreviewMode,
selectedPageStore,
Expand Down Expand Up @@ -55,6 +56,14 @@ const temporaryRootInstance: Instance = {
const useElementsTree = (getComponent: GetComponent) => {
const [rootInstance] = useRootInstance();

// @todo remove after https://github.com/webstudio-is/webstudio-builder/issues/1313 now its needed to be sure that no leaks exists
// eslint-disable-next-line no-console
console.log({
assetsStore: assetsStore.get().size,
pagesStore: pagesStore.get()?.pages.length ?? 0,
instancesStore: instancesStore.get().size,
});

const pagesMapStore = useMemo(
() =>
computed(pagesStore, (pages): Map<string, Page> => {
Expand Down
19 changes: 10 additions & 9 deletions apps/builder/app/shared/hook-utils/use-sync-initialize-once.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import { useRef } from "react";
import { useEffect, useRef } from "react";

// This hook allows to
// 1. run a function only once on the client
// 2. run it server-side
// 3. run the function synchronously in both
// Can't use effect for that (async).
// Can't use useState initializer because it can be called twice in strict mode and also isn't designed for this.
export const useSyncInitializeOnce = (fn: () => void) => {
const ref = useRef(false);
if (ref.current) {
return;
}
ref.current = true;
fn();

useEffect(() => {
if (ref.current) {
return;
}
ref.current = true;

fn();
}, [fn]);
};

0 comments on commit 13db987

Please sign in to comment.