fix: refactor mocks to its own project #284

yusinto · 2023-09-25T23:33:46Z

Large pr but mostly trivial changes to import paths because mocks have been moved to its own private project @launchdarkly/private-js-mocks:

Refactored mocks to its own project @launchdarkly/private-js-mocks.
Better eslint rule for no-unused-vars to avoid disabling this rule sporadically.

…c-217814/fix-mocks-and-jest-runtime-dependency

…orcing no-unused-vars.

shortcut-integration · 2023-09-25T23:33:50Z

This pull request has been linked to Shortcut Story #217814: Fix mocks and jest runtime dependency.

louis-launchdarkly · 2023-09-25T23:55:23Z

It may not be fully ready for review, but creating a separate package and the change within it looks generally good to me.

packages/mocks/LICENSE

kinyoklion · 2023-09-26T16:39:20Z

Is this ready for review? I was added as a reviewer, but I do see it is draft.

yusinto · 2023-09-26T17:39:32Z

Not ready yet I need to fix a broken test and fix a circular dependency scenario which I want to improve.

… and use common-types.

…ommon.

yusinto · 2023-09-27T18:56:02Z

.eslintrc.js

+    '@typescript-eslint/no-unused-vars': [
+      'error',
+      { ignoreRestSiblings: true, argsIgnorePattern: '^_', varsIgnorePattern: '^__' },
+    ],


I was concerned with increased usage of disabling of this rule and so this is a more sustainable practice going forward.

yusinto · 2023-09-27T18:56:19Z

.eslintrc.js

+    'import/default': 'error',
+    'import/export': 'error',
+    'import/no-self-import': 'error',
+    'import/no-cycle': 'error',
+    'import/no-useless-path-segments': 'error',
+    'import/no-duplicates': 'error',


Additional sensible rules which are good to have.

yusinto · 2023-09-27T19:01:16Z

packages/shared/common/src/internal/index.ts

@@ -1,4 +1,3 @@
 export * from './diagnostics';
 export * from './events';
 export * from './stream';
-export * as mocks from './mocks';


Good riddance.

yusinto · 2023-09-27T19:03:24Z

packages/shared/common/tsconfig.json

@@ -12,7 +12,8 @@
    "sourceMap": true,
    "declaration": true,
    "declarationMap": true, // enables importers to jump to source
-    "stripInternal": true
+    "stripInternal": true,
+    "composite": true


mocks need types from common. This composite allows common to be referred to in mocks so we can import common types.

yusinto · 2023-09-27T19:06:41Z

packages/shared/mocks/package.json

+  "name": "@launchdarkly/private-js-mocks",
+  "private": true,


We are not publishing mocks to npm because it's internal use only.

I'm open to improving the name of this package. It's the best I can come up with right now.

yusinto · 2023-09-27T19:09:32Z

packages/shared/mocks/src/index.ts

@@ -10,6 +11,7 @@ export {
  crypto,
  logger,
  hasher,
+  ContextDeduplicator,


There was a bug before this PR where this export was missing. This is to fix that bug.

yusinto · 2023-09-27T19:12:14Z

packages/shared/mocks/tsconfig.json

+  "exclude": ["**/*.test.ts", "dist", "node_modules", "__tests__"],
+  "references": [
+    {
+      "path": "../common"


This enables us to import types from common. Then on line 17 above, we add an alias so we can import common without relative paths to appease eslint.

So this means that mocks specifically will only work when checked out as a monorepo? Or will these things get bundled into the dist?

Mocks will only work when checked out as monorepo. There's no bundling to produce any bundles for mocks. The dist folder only contains tsc output without bundling and does not contain any common code.

Just to re-iterate, the mocks projects are only using types from common and nothing else.

yusinto · 2023-09-27T19:14:33Z

packages/shared/sdk-server/src/LDClientImpl.ts

@@ -25,7 +23,7 @@ import DataSourceUpdates from './data_sources/DataSourceUpdates';
 import PollingProcessor from './data_sources/PollingProcessor';
 import Requestor from './data_sources/Requestor';
 import createDiagnosticsInitConfig from './diagnostics/createDiagnosticsInitConfig';
-import { allAsync, allSeriesAsync } from './evaluation/collection';


allSeriesAsync is removed because it was unused.

yusinto · 2023-09-27T21:16:53Z

packages/shared/sdk-server/src/LDClientImpl.ts

@@ -485,11 +481,9 @@ export default class LDClientImpl implements LDClient {
    this.variationInternal(flagKey, context, defaultValue, eventFactory, cb);
  }

-  private dataSourceErrorHandler(e: LDStreamingError | LDPollingError) {


Relax the error type here to allow all kinds of errors to be handled. This allows mock errors to be handled.

kinyoklion

This is probably all good, but I think we are going to need to make some packages to be sure.

yusinto added 5 commits September 19, 2023 14:46

fix: add getJest function to check jest existence

18edb54

Merge branch 'yus/sc-208024/scaffold-js-common-in-js-core' into yus/s…

5139935

…c-217814/fix-mocks-and-jest-runtime-dependency

chore: fix eslint error unused args

1fff2a3

chore: moved internal/mocks to its own project. be stricter about enf…

483c0f4

…orcing no-unused-vars.

chore: add sensible good-to-have eslint import rules.

b51c3e6

yusinto requested review from kinyoklion and louis-launchdarkly as code owners September 25, 2023 23:33

yusinto marked this pull request as draft September 25, 2023 23:33

kinyoklion reviewed Sep 26, 2023

View reviewed changes

packages/mocks/LICENSE Outdated Show resolved Hide resolved

yusinto added 3 commits September 26, 2023 22:09

chore: move common types to its own project. moved mocks under shared…

741010e

… and use common-types.

chore: deleted common-types replaced with tsconfig references to ../c…

c3564ca

…ommon.

chore: replace relative path to common with alias

32430c4

yusinto commented Sep 27, 2023

View reviewed changes

yusinto added 6 commits September 27, 2023 12:15

chore: minor clean up

16535b9

chore: added ci steps and improved readme.

aa4b314

chore: remove jest command.

8509fe2

chore: remove extraneous underscore

f15a336

chore: removed redundant ts-ignore on ContextDeduplicator.

14ba531

chore: relax error type to allow all kinds of errors to be handled.

cc4cac4

yusinto commented Sep 27, 2023

View reviewed changes

yusinto marked this pull request as ready for review September 27, 2023 21:17

yusinto requested a review from kinyoklion September 27, 2023 21:17

yusinto added the move-js-common label Sep 27, 2023

kinyoklion approved these changes Sep 27, 2023

View reviewed changes

yusinto requested a review from kinyoklion October 2, 2023 18:31

kinyoklion approved these changes Oct 2, 2023

View reviewed changes

yusinto merged commit 2ada20d into yus/sc-208024/scaffold-js-common-in-js-core Oct 2, 2023
2 checks passed

yusinto deleted the yus/sc-217814/fix-mocks-and-jest-runtime-dependency branch October 2, 2023 19:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: refactor mocks to its own project #284

fix: refactor mocks to its own project #284

yusinto commented Sep 25, 2023 •

edited

Loading

shortcut-integration bot commented Sep 25, 2023

louis-launchdarkly commented Sep 25, 2023

kinyoklion commented Sep 26, 2023

yusinto commented Sep 26, 2023 •

edited

Loading

yusinto Sep 27, 2023

yusinto Sep 27, 2023

yusinto Sep 27, 2023

yusinto Sep 27, 2023

yusinto Sep 27, 2023

yusinto Sep 27, 2023

yusinto Sep 27, 2023

kinyoklion Sep 27, 2023

yusinto Oct 2, 2023

yusinto Sep 27, 2023

yusinto Sep 27, 2023

kinyoklion left a comment

fix: refactor mocks to its own project #284

fix: refactor mocks to its own project #284

Conversation

yusinto commented Sep 25, 2023 • edited Loading

shortcut-integration bot commented Sep 25, 2023

louis-launchdarkly commented Sep 25, 2023

kinyoklion commented Sep 26, 2023

yusinto commented Sep 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kinyoklion left a comment

Choose a reason for hiding this comment

yusinto commented Sep 25, 2023 •

edited

Loading

yusinto commented Sep 26, 2023 •

edited

Loading