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

feat: Apply private property naming standard. Mangle browser private properties. #620

Merged
merged 5 commits into from
Oct 10, 2024
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
32 changes: 32 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,38 @@ module.exports = {
'jest/no-focused-tests': 'error',
'jest/no-identical-title': 'error',
'jest/valid-expect': 'error',
'no-underscore-dangle': ['error', { allowAfterThis: true }],
'@typescript-eslint/naming-convention': [
Copy link
Member Author

Choose a reason for hiding this comment

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

Basically these go broad and then specific. Forbid underscore!... unless private and then require. And again for the properties.

'error',
{
selector: ['method'],
format: ['camelCase'],
leadingUnderscore: 'forbid',
},
{
selector: ['method'],
format: ['camelCase'],
modifiers: ['private'],
leadingUnderscore: 'require',
},
{
selector: ['classProperty', 'parameterProperty'],
format: ['camelCase'],
leadingUnderscore: 'forbid',
},
{
selector: ['classProperty', 'parameterProperty'],
modifiers: ['static'],
format: ['PascalCase'],
leadingUnderscore: 'forbid',
},
{
selector: ['classProperty', 'parameterProperty'],
modifiers: ['private'],
format: ['camelCase'],
leadingUnderscore: 'require',
},
],
},
globals: {
BigInt: 'readonly',
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/release-please.yml
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ jobs:
workspace_path: packages/sdk/browser
aws_assume_role: ${{ vars.AWS_ROLE_ARN }}


release-server-node:
runs-on: ubuntu-latest
needs: ['release-please', 'release-sdk-server']
Expand Down
6 changes: 3 additions & 3 deletions packages/sdk/akamai-edgekv/src/edgekv/edgeKVProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ type EdgeKVProviderParams = {
};

export default class EdgeKVProvider implements EdgeProvider {
private edgeKv: EdgeKV;
private _edgeKv: EdgeKV;

constructor({ namespace, group }: EdgeKVProviderParams) {
this.edgeKv = new EdgeKV({ namespace, group } as any);
this._edgeKv = new EdgeKV({ namespace, group } as any);
}

async get(rootKey: string): Promise<string | null | undefined> {
try {
return await this.edgeKv.getText({ item: rootKey } as any);
return await this._edgeKv.getText({ item: rootKey } as any);
} catch (e) {
/* empty */
}
Expand Down
1 change: 1 addition & 0 deletions packages/sdk/browser/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
-->

# ⛔️⛔️⛔️⛔️

Copy link
Member Author

Choose a reason for hiding this comment

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

Lint.

> [!CAUTION]
> This library is a alpha version and should not be considered ready for production use while this message is visible.

Expand Down
37 changes: 20 additions & 17 deletions packages/sdk/browser/contract-tests/entity/src/ClientEntity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,17 @@ function makeDefaultInitialContext() {

export class ClientEntity {
constructor(
private readonly client: LDClient,
private readonly logger: LDLogger,
private readonly _client: LDClient,
private readonly _logger: LDLogger,
) {}

close() {
this.client.close();
this.logger.info('Test ended');
this._client.close();
this._logger.info('Test ended');
}

async doCommand(params: CommandParams) {
this.logger.info(`Received command: ${params.command}`);
this._logger.info(`Received command: ${params.command}`);
switch (params.command) {
case CommandType.EvaluateFlag: {
const evaluationParams = params.evaluate;
Expand All @@ -95,23 +95,23 @@ export class ClientEntity {
if (evaluationParams.detail) {
switch (evaluationParams.valueType) {
case ValueType.Bool:
return this.client.boolVariationDetail(
return this._client.boolVariationDetail(
evaluationParams.flagKey,
evaluationParams.defaultValue as boolean,
);
case ValueType.Int: // Intentional fallthrough.
case ValueType.Double:
return this.client.numberVariationDetail(
return this._client.numberVariationDetail(
evaluationParams.flagKey,
evaluationParams.defaultValue as number,
);
case ValueType.String:
return this.client.stringVariationDetail(
return this._client.stringVariationDetail(
evaluationParams.flagKey,
evaluationParams.defaultValue as string,
);
default:
return this.client.variationDetail(
return this._client.variationDetail(
evaluationParams.flagKey,
evaluationParams.defaultValue,
);
Expand All @@ -120,42 +120,45 @@ export class ClientEntity {
switch (evaluationParams.valueType) {
case ValueType.Bool:
return {
value: this.client.boolVariation(
value: this._client.boolVariation(
evaluationParams.flagKey,
evaluationParams.defaultValue as boolean,
),
};
case ValueType.Int: // Intentional fallthrough.
case ValueType.Double:
return {
value: this.client.numberVariation(
value: this._client.numberVariation(
evaluationParams.flagKey,
evaluationParams.defaultValue as number,
),
};
case ValueType.String:
return {
value: this.client.stringVariation(
value: this._client.stringVariation(
evaluationParams.flagKey,
evaluationParams.defaultValue as string,
),
};
default:
return {
value: this.client.variation(evaluationParams.flagKey, evaluationParams.defaultValue),
value: this._client.variation(
evaluationParams.flagKey,
evaluationParams.defaultValue,
),
};
}
}

case CommandType.EvaluateAllFlags:
return { state: this.client.allFlags() };
return { state: this._client.allFlags() };

case CommandType.IdentifyEvent: {
const identifyParams = params.identifyEvent;
if (!identifyParams) {
throw malformedCommand;
}
await this.client.identify(identifyParams.user || identifyParams.context);
await this._client.identify(identifyParams.user || identifyParams.context);
return undefined;
}

Expand All @@ -164,7 +167,7 @@ export class ClientEntity {
if (!customEventParams) {
throw malformedCommand;
}
this.client.track(
this._client.track(
customEventParams.eventKey,
customEventParams.data,
customEventParams.metricValue,
Expand All @@ -173,7 +176,7 @@ export class ClientEntity {
}

case CommandType.FlushEvents:
this.client.flush();
this._client.flush();
return undefined;

default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,31 @@ import { ClientEntity, newSdkClientEntity } from './ClientEntity';
import { makeLogger } from './makeLogger';

export default class TestHarnessWebSocket {
private ws?: WebSocket;
private readonly entities: Record<string, ClientEntity> = {};
private clientCounter = 0;
private logger: LDLogger = makeLogger('TestHarnessWebSocket');
private _ws?: WebSocket;
private readonly _entities: Record<string, ClientEntity> = {};
private _clientCounter = 0;
private _logger: LDLogger = makeLogger('TestHarnessWebSocket');

constructor(private readonly url: string) {}
constructor(private readonly _url: string) {}

connect() {
this.logger.info(`Connecting to web socket.`);
this.ws = new WebSocket(this.url, ['v1']);
this.ws.onopen = () => {
this.logger.info('Connected to websocket.');
this._logger.info(`Connecting to web socket.`);
this._ws = new WebSocket(this._url, ['v1']);
this._ws.onopen = () => {
this._logger.info('Connected to websocket.');
};
this.ws.onclose = () => {
this.logger.info('Websocket closed. Attempting to reconnect in 1 second.');
this._ws.onclose = () => {
this._logger.info('Websocket closed. Attempting to reconnect in 1 second.');
setTimeout(() => {
this.connect();
}, 1000);
};
this.ws.onerror = (err) => {
this.logger.info(`error:`, err);
this._ws.onerror = (err) => {
this._logger.info(`error:`, err);
};

this.ws.onmessage = async (msg) => {
this.logger.info('Test harness message', msg);
this._ws.onmessage = async (msg) => {
this._logger.info('Test harness message', msg);
const data = JSON.parse(msg.data);
const resData: any = { reqId: data.reqId };
switch (data.command) {
Expand All @@ -46,33 +46,33 @@ export default class TestHarnessWebSocket {
break;
case 'createClient':
{
resData.resourceUrl = `/clients/${this.clientCounter}`;
resData.resourceUrl = `/clients/${this._clientCounter}`;
resData.status = 201;
const entity = await newSdkClientEntity(data.body);
this.entities[this.clientCounter] = entity;
this.clientCounter += 1;
this._entities[this._clientCounter] = entity;
this._clientCounter += 1;
}
break;
case 'runCommand':
if (Object.prototype.hasOwnProperty.call(this.entities, data.id)) {
const entity = this.entities[data.id];
if (Object.prototype.hasOwnProperty.call(this._entities, data.id)) {
const entity = this._entities[data.id];
const body = await entity.doCommand(data.body);
resData.body = body;
resData.status = body ? 200 : 204;
} else {
resData.status = 404;
this.logger.warn(`Client did not exist: ${data.id}`);
this._logger.warn(`Client did not exist: ${data.id}`);
}

break;
case 'deleteClient':
if (Object.prototype.hasOwnProperty.call(this.entities, data.id)) {
const entity = this.entities[data.id];
if (Object.prototype.hasOwnProperty.call(this._entities, data.id)) {
const entity = this._entities[data.id];
entity.close();
delete this.entities[data.id];
delete this._entities[data.id];
} else {
resData.status = 404;
this.logger.warn(`Could not delete client because it did not exist: ${data.id}`);
this._logger.warn(`Could not delete client because it did not exist: ${data.id}`);
}
break;
default:
Expand All @@ -84,10 +84,10 @@ export default class TestHarnessWebSocket {
}

disconnect() {
this.ws?.close();
this._ws?.close();
}

send(data: unknown) {
this.ws?.send(JSON.stringify(data));
this._ws?.send(JSON.stringify(data));
}
}
22 changes: 20 additions & 2 deletions packages/sdk/browser/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,32 @@ export default [
esmExternals: true,
}),
resolve(),
terser(),
terser({
mangle: {
properties: {
regex: /^_/,
},
},
}),
json(),
// The 'sourcemap' option allows using the minified size, not the size before minification.
visualizer({ sourcemap: true }),
],
},
{
...getSharedConfig('cjs', 'dist/index.cjs.js'),
plugins: [typescript(), common(), resolve(), terser(), json()],
plugins: [
Copy link
Member Author

Choose a reason for hiding this comment

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

Lint.

typescript(),
common(),
resolve(),
terser({
mangle: {
properties: {
regex: /^_/,
},
},
}),
json(),
],
},
];
16 changes: 8 additions & 8 deletions packages/sdk/browser/src/BrowserClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,10 @@ export type LDClient = Omit<
};

export class BrowserClient extends LDClientImpl implements LDClient {
private readonly goalManager?: GoalManager;
private readonly _goalManager?: GoalManager;

constructor(
private readonly clientSideId: string,
clientSideId: string,
autoEnvAttributes: AutoEnvAttributes,
options: BrowserOptions = {},
overridePlatform?: Platform,
Expand Down Expand Up @@ -174,7 +174,7 @@ export class BrowserClient extends LDClientImpl implements LDClient {
this.setEventSendingEnabled(true, false);

if (validatedBrowserOptions.fetchGoals) {
this.goalManager = new GoalManager(
this._goalManager = new GoalManager(
clientSideId,
platform.requests,
baseUrl,
Expand Down Expand Up @@ -215,7 +215,7 @@ export class BrowserClient extends LDClientImpl implements LDClient {
// "waitForGoalsReady", then we would make an async immediately invoked function expression
// which emits the event, and assign its promise to a member. The "waitForGoalsReady" function
// would return that promise.
this.goalManager.initialize();
this._goalManager.initialize();

if (validatedBrowserOptions.automaticBackgroundHandling) {
registerStateDetection(() => this.flush());
Expand All @@ -225,7 +225,7 @@ export class BrowserClient extends LDClientImpl implements LDClient {

override async identify(context: LDContext, identifyOptions?: LDIdentifyOptions): Promise<void> {
await super.identify(context, identifyOptions);
this.goalManager?.startTracking();
this._goalManager?.startTracking();
}

setStreaming(streaming?: boolean): void {
Expand All @@ -235,7 +235,7 @@ export class BrowserClient extends LDClientImpl implements LDClient {
browserDataManager.setForcedStreaming(streaming);
}

private updateAutomaticStreamingState() {
private _updateAutomaticStreamingState() {
const browserDataManager = this.dataManager as BrowserDataManager;
// This will need changed if support for listening to individual flag change
// events it added.
Expand All @@ -244,11 +244,11 @@ export class BrowserClient extends LDClientImpl implements LDClient {

override on(eventName: LDEmitterEventName, listener: Function): void {
super.on(eventName, listener);
this.updateAutomaticStreamingState();
this._updateAutomaticStreamingState();
}

override off(eventName: LDEmitterEventName, listener: Function): void {
super.off(eventName, listener);
this.updateAutomaticStreamingState();
this._updateAutomaticStreamingState();
}
}
Loading
Loading