Skip to content
Merged
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
28 changes: 15 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,21 @@ if (prefab.isEnabled('cool-feature') {
setTimeout(ping, prefab.get('ping-delay'));
```

Here's an explanation of each property

| property | example | purpose |
| --------------- | ------------------------------------ | -------------------------------------------------------------------------------------------- |
| `isEnabled` | `prefab.isEnabled("new-logo")` | returns a boolean (default `false`) if a feature is enabled based on the current context |
| `get` | `prefab.get('retry-count')` | returns the value of a flag or config evaluated in the current context |
| `getDuration` | `prefab.getDuration('http.timeout')` | returns a duration object `{seconds: number, ms: number}` |
| `loaded` | `if (prefab.loaded) { ... }` | a boolean indicating whether prefab content has loaded |
| `shouldLog` | `if (prefab.shouldLog(...)) {` | returns a boolean indicating whether the proposed log level is valid for the current context |
| `poll` | `prefab.poll({frequencyInMs})` | starts polling every `frequencyInMs` ms. |
| `stopPolling` | `prefab.stopPolling()` | stops the polling process |
| `context` | `prefab.context` | get the current context (after `init()`). |
| `updateContext` | `prefab.updateContext(newContext)` | update the context and refetch. Pass `false` as a second argument to skip refetching |
## Client API

| property | example | purpose |
| --------------- | ------------------------------------- | -------------------------------------------------------------------------------------------- |
| `isEnabled` | `prefab.isEnabled("new-logo")` | returns a boolean (default `false`) if a feature is enabled based on the current context |
| `get` | `prefab.get('retry-count')` | returns the value of a flag or config evaluated in the current context |
| `getDuration` | `prefab.getDuration('http.timeout')` | returns a duration object `{seconds: number, ms: number}` |
| `loaded` | `if (prefab.loaded) { ... }` | a boolean indicating whether prefab content has loaded |
| `shouldLog` | `if (prefab.shouldLog(...)) {` | returns a boolean indicating whether the proposed log level is valid for the current context |
| `poll` | `prefab.poll({frequencyInMs})` | starts polling every `frequencyInMs` ms. |
| `stopPolling` | `prefab.stopPolling()` | stops the polling process |
| `context` | `prefab.context` | get the current context (after `init()`). |
| `updateContext` | `prefab.updateContext(newContext)` | update the context and refetch. Pass `false` as a second argument to skip refetching |
| `extract` | `prefab.extract()` | returns the current config as a plain object of key, config value pairs |
| `hydrate` | `prefab.hydrate(configurationObject)` | sets the current config based on a plain object of key, config value pairs |

## `shouldLog()`

Expand Down
8 changes: 4 additions & 4 deletions src/afterEvaluationCallback.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe("afterEvaluationCallback", () => {
const prefab = new Prefab();
prefab.afterEvaluationCallback = callback;

prefab.setConfig({ turbo: 2.5 });
prefab.hydrate({ turbo: 2.5 });

expect(callback).not.toHaveBeenCalled();

Expand All @@ -46,7 +46,7 @@ describe("afterEvaluationCallback", () => {

prefab.afterEvaluationCallback = callback;

prefab.setConfig({ turbo: 2.5 });
prefab.hydrate({ turbo: 2.5 });

expect(callback).not.toHaveBeenCalled();

Expand All @@ -72,7 +72,7 @@ describe("afterEvaluationCallback", () => {
await waitForAsyncCall();
expect(callback).toHaveBeenCalledTimes(0);

prefab.setConfig({ foo: true });
prefab.hydrate({ foo: true });

expect(prefab.isEnabled("foo")).toBe(true);

Expand All @@ -94,7 +94,7 @@ describe("afterEvaluationCallback", () => {
await waitForAsyncCall();
expect(callback).toHaveBeenCalledTimes(0);

prefab.setConfig({ foo: true });
prefab.hydrate({ foo: true });

expect(prefab.isEnabled("foo")).toBe(true);

Expand Down
38 changes: 30 additions & 8 deletions src/prefab.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,11 @@ describe("poll", () => {
});
});

describe("setConfig", () => {
describe("hydrate", () => {
it("works when types are not provided", () => {
expect(prefab.configs).toEqual({});

prefab.setConfig({
prefab.hydrate({
turbo: 2.5,
foo: true,
jsonExample: { foo: "bar", baz: 123 },
Expand Down Expand Up @@ -285,7 +285,7 @@ describe("bootstrapping", () => {
});

test("get", () => {
prefab.setConfig({
prefab.hydrate({
evaluations: {
turbo: { value: { double: 2.5 } },
durationExample: { value: { duration: { millis: 1884000, definition: "PT1884S" } } },
Expand All @@ -308,7 +308,7 @@ test("get", () => {
});

test("getDuration", () => {
prefab.setConfig({
prefab.hydrate({
evaluations: {
turbo: { value: { double: 2.5 } },
durationExample: {
Expand All @@ -331,11 +331,33 @@ test("isEnabled", () => {
// it is false when no config is loaded
expect(prefab.isEnabled("foo")).toBe(false);

prefab.setConfig({ foo: true });
prefab.hydrate({ foo: true });

expect(prefab.isEnabled("foo")).toBe(true);
});

describe("extract", () => {
it("correctly extracts configuration values", () => {
prefab.hydrate({
turbo: 2.5,
foo: true,
jsonExample: { foo: "bar", baz: 123 },
});

const extracted = prefab.extract();
expect(extracted).toEqual({
turbo: 2.5,
foo: true,
jsonExample: { foo: "bar", baz: 123 },
});
});

it("returns an empty object when no configs are set", () => {
const extracted = prefab.extract();
expect(extracted).toEqual({});
});
});

describe("shouldLog", () => {
test("compares against the default level where there is no value", () => {
expect(
Expand All @@ -356,7 +378,7 @@ describe("shouldLog", () => {
});

test("compares against the value when present", () => {
prefab.setConfig({
prefab.hydrate({
"log-level.example": "INFO",
});

Expand All @@ -380,7 +402,7 @@ describe("shouldLog", () => {
test("traverses the hierarchy to get the closest level for the loggerName", () => {
const loggerName = "some.test.name.with.more.levels";

prefab.setConfig({
prefab.hydrate({
"log-level.some.test.name": "TRACE",
"log-level.some.test": "DEBUG",
"log-level.irrelevant": "ERROR",
Expand Down Expand Up @@ -420,7 +442,7 @@ describe("shouldLog", () => {
});

it("can use the root log level setting if nothing is found in the hierarchy", () => {
prefab.setConfig({
prefab.hydrate({
"log-level": "INFO",
});

Expand Down
32 changes: 28 additions & 4 deletions src/prefab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,24 @@ export class Prefab {
return this.load();
}

get configs(): { [key: string]: Config } {
extract(): Record<string, Config["value"]> {
Comment thread
mjfaga marked this conversation as resolved.
return Object.entries(this._configs).reduce(
(agg, [key, value]) => ({
...agg,
[key]: value.value,
}),
{} as Record<string, Config["value"]>
);
}

hydrate(rawValues: RawConfigWithoutTypes | EvaluationPayload): void {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure there is an actual use-case for supporting EvaluationPayload. Hoping someone can give me some guidance here with regards to where this particular payload comes from and if we need to support it in the hydration lifecycle.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe EvaluationPayload is basically what we pass back to the telemetry endpoints. It basically has the coordinates of the eval, so we can say "10% of users hit this rule in your flag" to build the charts.

The trick for something like this is understandable if you think of experiment evaluation. You might want to only send the exposure event when the actual flag is evaluated, but SSR you're going to eval everything in order to give values to the front end. The customer isn't really "evaluating" all those flags, so we don't want to fire telemetry as SSR hydration time, we really should fire the telemetry when/if the actually prefab.get() is called. But same thing for trying to think about zombie flag detection. you don't want to say "all these flags are in use" just because you're hydrating.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

that all makes sense... That said, in the context of this change, we aren't considering them "evaluated" as far as I can tell. What I implement hydrates the cache using the same this.setConfig (now private) method.

It's also worth calling out that SSR may have evaluated some of the flags in the cache. In that case, I'm assuming you guys already handle flushing the telemetry, and there is no need to pass it to the FE to ensure it's flushed.

Otherwise, I agree there is no need to support the evaluations API here, in which case I will update the hydrate method to explicitly not support that, both in typing + in implementation.

Thoughts?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I investigated a bit further based on what comes back from the API when initializing the client as well as how evaluations are tracked and shipped. The stuff in the explicit cache actually doesn't include any of the actual evaluation data. That is tracked in a different data object entirely. So there is no harm in actually supporting the evaluation payload as it exists in the cache.

This raises more questions for me than anything in terms of why this is actually needed in the cache at all. We can have that conversation another day.

this.setConfigPrivate(rawValues);
}

get configs(): Record<string, Config> {
// Log message in yellow without adding chalk dependency
console.warn("\x1b[33m%s\x1b[0m", 'Deprecated: Use "prefab.extract" instead');

return this._configs;
}

Expand Down Expand Up @@ -174,7 +191,7 @@ export class Prefab {
const bootstrapContext = new Context(prefabBootstrap.context);

if (this.context.equals(bootstrapContext)) {
this.setConfig({ evaluations: prefabBootstrap.evaluations });
this.setConfigPrivate({ evaluations: prefabBootstrap.evaluations });
return Promise.resolve();
}
}
Expand All @@ -185,7 +202,7 @@ export class Prefab {
return this.loader
.load()
.then((rawValues: any) => {
this.setConfig(rawValues as EvaluationPayload);
this.setConfigPrivate(rawValues as EvaluationPayload);
})
.finally(() => {
if (this.pollStatus.status === "running") {
Expand Down Expand Up @@ -254,6 +271,13 @@ export class Prefab {
}

setConfig(rawValues: RawConfigWithoutTypes | EvaluationPayload) {
// Log message in yellow without adding chalk dependency
console.warn("\x1b[33m%s\x1b[0m", 'Deprecated: Use "prefab.hydrate" instead');

this.setConfigPrivate(rawValues);
}

private setConfigPrivate(rawValues: RawConfigWithoutTypes | EvaluationPayload) {
this._configs = Config.digest(rawValues);
this.loaded = true;
}
Expand All @@ -274,7 +298,7 @@ export class Prefab {
return undefined;
}

const config = this.configs[key];
const config = this._configs[key];

const value = config?.value;

Expand Down