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
39 changes: 39 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions packages/compass-telemetry/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,26 @@ module.exports = {
tsconfigRootDir: __dirname,
project: ['./tsconfig-lint.json'],
},
overrides: [
Copy link
Collaborator

Choose a reason for hiding this comment

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

[nit]: if this package is intended to be used within compass-telemetry only then it makes sense to add it here, or else you can add it to base eslint config file: https://github.com/mongodb-js/compass/blob/main/configs/eslint-config-compass/index.js

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think we'll need these types outside this package! We'll import useAssignment or assignExperiment, but I don't think we'll have to define any other objects with the types.

{
files: ['./src/**/*.ts', './src/**/*.tsx'],
rules: {
'no-restricted-imports': 'off',
'@typescript-eslint/no-restricted-imports': [
'error',
{
paths: [
{
name: '@mongodb-js/mdb-experiment-js',
message:
'Use type-only imports from @mongodb-js/mdb-experiment-js',
allowTypeImports: true,
},
],
},
],
'@typescript-eslint/no-redundant-type-constituents': 'off',
},
},
],
};
3 changes: 2 additions & 1 deletion packages/compass-telemetry/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@
"@mongodb-js/compass-logging": "^1.7.9",
"@mongodb-js/compass-app-registry": "^9.4.18",
"hadron-ipc": "^3.5.8",
"react": "^17.0.2"
"react": "^17.0.2",
"@mongodb-js/mdb-experiment-js": "1.9.0"
},
"devDependencies": {
"@mongodb-js/eslint-config-compass": "^1.4.5",
Expand Down
62 changes: 62 additions & 0 deletions packages/compass-telemetry/src/experimentation-provider.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import React, { createContext, useContext, useRef } from 'react';
import type { types } from '@mongodb-js/mdb-experiment-js';
import type { typesReact } from '@mongodb-js/mdb-experiment-js/react';

type UseAssignmentHook = (
experimentName: string,
trackIsInSample: boolean,
options?: typesReact.UseAssignmentOptions<types.TypeData>
) => typesReact.UseAssignmentResponse<types.TypeData>;

type AssignExperimentFn = (
experimentName: string,
options?: types.AssignOptions<string>
) => Promise<types.AsyncStatus | null>;
Comment on lines +5 to +14
Copy link

@diiiefiend diiiefiend Jul 31, 2025

Choose a reason for hiding this comment

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

Do we want to note that these should be kept in sync with the signatures in the SDK (with a link to where the SDK defines these)? I assume so, since these values are coming from MMS, which will be following the SDK conventions.

This does feel a little brittle, but it'll prob do for now til if/when we make the SDK a first-order dep in this repo.

Copy link
Collaborator Author

@jcobis jcobis Jul 31, 2025

Choose a reason for hiding this comment

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

My understanding is that the parent type signatures (UseAssignmentHook and AssignExperimentFn) are actually defined in MMS to type the useAssignment and assign functions respectively (to match the SDK functions), and that MMS uses the SDK types (typesReact.UseAssignmentOptions, typesReact.UseAssignmentResponse, etc.) as elements in those parent type signatures, but the parent type signatures are not exported by the SDK or MMS. Is that right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So should we link to the MMS or SDK useAssignment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Going to merge this in to unblock tickets and add the comment on next compass PR


interface CompassExperimentationProviderContextValue {
useAssignment: UseAssignmentHook;
assignExperiment: AssignExperimentFn;
}

const initialContext: CompassExperimentationProviderContextValue = {
useAssignment() {
return {
assignment: null,
asyncStatus: null,
error: null,
isLoading: false,
isError: false,
isSuccess: true,
};
},
assignExperiment() {
return Promise.resolve(null);
},
};

const ExperimentationContext =
createContext<CompassExperimentationProviderContextValue>(initialContext);

// Provider component that accepts MMS experiment utils as props
export const CompassExperimentationProvider: React.FC<{
children: React.ReactNode;
useAssignment: UseAssignmentHook;
assignExperiment: AssignExperimentFn;
}> = ({ children, useAssignment, assignExperiment }) => {
// Use useRef to keep the functions up-to-date; Use mutation pattern to maintain the
// same object reference to prevent unnecessary re-renders of consuming components
const { current: contextValue } = useRef({ useAssignment, assignExperiment });
contextValue.useAssignment = useAssignment;
contextValue.assignExperiment = assignExperiment;
Comment on lines +48 to +50
Copy link
Preview

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

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

The useRef pattern here won't prevent re-renders as intended. The context value object is recreated on every render because it captures the current prop values. Consider using useMemo with proper dependencies instead: const contextValue = useMemo(() => ({ useAssignment, assignExperiment }), [useAssignment, assignExperiment]);

Suggested change
const { current: contextValue } = useRef({ useAssignment, assignExperiment });
contextValue.useAssignment = useAssignment;
contextValue.assignExperiment = assignExperiment;
const contextValue = React.useMemo(
() => ({ useAssignment, assignExperiment }),
[useAssignment, assignExperiment]
);

Copilot uses AI. Check for mistakes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ignoring this, see above discussion


return (
<ExperimentationContext.Provider value={contextValue}>
{children}
</ExperimentationContext.Provider>
);
};

// Hook for components to access experiment assignment
export const useAssignment = (...args: Parameters<UseAssignmentHook>) => {
return useContext(ExperimentationContext).useAssignment(...args);
};
2 changes: 2 additions & 0 deletions packages/compass-telemetry/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ export type {
IdentifyTraits,
ExtraConnectionData,
} from './types';

export { CompassExperimentationProvider } from './experimentation-provider';
2 changes: 2 additions & 0 deletions packages/compass-web/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@ export type {
OpenWorkspaceOptions,
WorkspaceTab,
} from '@mongodb-js/compass-workspaces';

export { CompassExperimentationProvider } from '@mongodb-js/compass-telemetry';
Loading