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

Add GC tests #176

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
15 changes: 15 additions & 0 deletions packages/signal-polyfill/src/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"compilerOptions": {
"rootDir": "./",
"pretty": true,
"moduleResolution": "node",
"module": "ESNext",
"target": "ES2022",
"noEmit": true,
"lib": ["DOM", "ES2021"],
"strict": true,
"composite": true,
"forceConsistentCasingInFileNames": true
},
"include": ["**/*.ts"]
}
59 changes: 55 additions & 4 deletions packages/signal-polyfill/src/wrapper.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -484,11 +484,11 @@ describe("Errors", () => {

describe("Cycles", () => {
it("detects trivial cycles", () => {
const c = new Signal.Computed(() => c.get());
const c: Signal.Computed<never> = new Signal.Computed(() => c.get());
Copy link
Collaborator

Choose a reason for hiding this comment

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

did something go wrong with inference here?

Copy link
Author

Choose a reason for hiding this comment

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

Yes it's an infinite loop c is depending on itself. TS wasn't being ran in strict mode before this PR, as the tests are excluded from the main tsconfig.

expect(() => c.get()).toThrow();
});
it("detects slightly larger cycles", () => {
const c = new Signal.Computed(() => c2.get());
const c: Signal.Computed<never> = new Signal.Computed(() => c2.get());
const c2 = new Signal.Computed(() => c.get());
const c3 = new Signal.Computed(() => c2.get());
expect(() => c3.get()).toThrow();
Expand Down Expand Up @@ -706,7 +706,7 @@ describe("Custom equality", () => {
describe("Receivers", () => {
it("is this for computed", () => {
let receiver;
const c = new Signal.Computed(function () {
const c = new Signal.Computed(function (this: Signal.Computed<void>) {
receiver = this;
});
expect(c.get()).toBe(undefined);
Expand Down Expand Up @@ -752,7 +752,7 @@ describe("Receivers", () => {
});

describe("Dynamic dependencies", () => {
function run(live) {
function run(live: boolean) {
const states = Array.from("abcdefgh").map((s) => new Signal.State(s));
const sources = new Signal.State(states);
const computed = new Signal.Computed(() => {
Expand Down Expand Up @@ -978,6 +978,7 @@ describe("type checks", () => {
TypeError,
);
expect(Signal.subtle.Watcher.prototype.watch.call(w, s)).toBe(undefined);
// @ts-expect-error
Copy link
Collaborator

@NullVoxPopuli NullVoxPopuli Apr 21, 2024

Choose a reason for hiding this comment

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

why expect error?

whenever I use ts-expect-error, I like to add a comment explaining why (not what (or the specific error -- the specific error is not relevant, but the why it exists is)!)

Copy link
Author

Choose a reason for hiding this comment

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

If you look below there's a bunch more pre-existing ts-expect-errors. Asides from that I think it's self explanatory in these cases why all of these are present. It's specifically testing areas with incorrect types to produce TypeErrors.

expect(() => Signal.subtle.Watcher.prototype.watch.call(w, w)).toThrowError(
TypeError,
);
Expand All @@ -993,18 +994,23 @@ describe("type checks", () => {
).toThrowError(TypeError);
expect(Signal.subtle.Watcher.prototype.unwatch.call(w, s)).toBe(undefined);
expect(() =>
// @ts-expect-error
Signal.subtle.Watcher.prototype.unwatch.call(w, w),
).toThrowError(TypeError);

expect(() =>
// @ts-expect-error
Signal.subtle.Watcher.prototype.getPending.call(x, s),
).toThrowError(TypeError);
expect(() =>
// @ts-expect-error
Signal.subtle.Watcher.prototype.getPending.call(s, s),
).toThrowError(TypeError);
expect(() =>
// @ts-expect-error
Signal.subtle.Watcher.prototype.getPending.call(c, s),
).toThrowError(TypeError);
// @ts-expect-error
expect(Signal.subtle.Watcher.prototype.getPending.call(w, s)).toStrictEqual(
[],
);
Expand Down Expand Up @@ -1044,6 +1050,51 @@ describe("currentComputed", () => {
});
});

describe("Garbage Collection", () => {
it("collects out of scope consumers", async () => {
const a = new Signal.State(1);
let b: Signal.Computed<number> | undefined = new Signal.Computed(() =>
a.get()
);
b.get();
const weakB = new WeakRef(b);
b = undefined;
await new Promise<void>((resolve) => setTimeout(() => resolve(), 0));
global.gc!();

expect(weakB.deref()).toBeUndefined();
});
it("collects inactive dynamic sources", async () => {
let a = new Signal.State(1);
const b: Signal.Computed<number> | undefined = new Signal.Computed(() =>
a.get()
);
b.get();
const weakOldA = new WeakRef(a);
a = new Signal.State(1);
await new Promise<void>((resolve) => setTimeout(() => resolve(), 0));
global.gc!();

expect(weakOldA.deref()).toBeUndefined();
});
it("collects inactive dynamic sources after update", async () => {
let a = new Signal.State(1);
const b = new Signal.State(1);
const c: Signal.Computed<number> | undefined = new Signal.Computed(() =>
a.get() + b.get()
);
c.get();
const weakOldA = new WeakRef(a);
a = new Signal.State(1);
b.set(2);

await new Promise<void>((resolve) => setTimeout(() => resolve(), 0));
global.gc!();

expect(weakOldA.deref()).toBeUndefined();
});
});

// Some things which we're comfortable with not hitting in code coverage:
// - The code for the callbacks (for reading signals and running watches)
// - Paths around writes being prohibited during computed/effect
Expand Down
16 changes: 12 additions & 4 deletions packages/signal-polyfill/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,15 @@ export default defineConfig({
lib: {
entry,
formats: ["es"],
fileName: "index"
}
}
});
fileName: "index",
},
},
test: {
pool: "forks",
poolOptions: {
forks: {
execArgv: ["--expose_gc"],
},
},
},
});
Loading