Replace Lazy with LazyRef #141

freshgum-bubbles · 2023-11-28T03:44:11Z

Replace the useless Lazy function with a new
LazyRef function in contrib.

Unlike Lazy, the LazyRef function is actually...
well, lazy -- it creates a reference host, much like
TransientRefHost, which allows for creating
an instance of the given reference at any time.

changeset-bot · 2023-11-28T03:44:14Z

🦋 Changeset detected

Latest commit: c8889a9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@freshgum/typedi	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

freshgum-bubbles

A few notes to self...

freshgum-bubbles · 2023-12-08T05:29:44Z

src/container-instance.class.mts

@@ -1274,7 +1274,7 @@ export class ContainerInstance implements Disposable {
       * One important note is that the dependency *itself* is never resolved in the container.
       */
      const parameters = serviceMetadata.dependencies.map(resolvable => ({
-        id: this.resolveTypeWrapper(resolvable.typeWrapper),
+        id: resolvable.typeWrapper.eagerType,


The now-removed method featured additional error-checking, e.g. null checks. This should either be reimplemented or added to changeset, as this is a breaking change.

freshgum-bubbles · 2023-12-08T05:30:00Z

src/container-instance.class.mts

@@ -1350,14 +1350,13 @@ export class ContainerInstance implements Disposable {
   */
  private resolveResolvable(resolvable: Resolvable, guardBuiltIns: boolean): unknown {
    const { typeWrapper } = resolvable;
-
-    const identifier = this.resolveTypeWrapper(resolvable.typeWrapper);
+    const identifier = resolvable.typeWrapper.eagerType;


Same thing here...

freshgum-bubbles · 2023-12-08T05:31:29Z

src/utils/resolve-to-type-wrapper.util.mts

@@ -12,15 +12,6 @@ import { isTypeWrapper } from './is-type-wrapper.util.mjs';
 * @param target the class definition of the target of the decorator
 */
 export function resolveToTypeWrapper(typeOrIdentifier: AnyInjectIdentifier): TypeWrapper {
-  /**


Perhaps these comments should be re-added (but actually relevant in the context of the API changes)? For context, these haven't changed since typestack -- they would probably just confuse readers more than help them.

freshgum-bubbles · 2023-12-16T02:18:49Z

src/container-instance.class.mts

@@ -1396,6 +1395,8 @@ export class ContainerInstance implements Disposable {
       */
      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
      return typeWrapper.extract!(this, constraints ?? ResolutionConstraintFlag.None);
+    } else if (!identifier) {


Probably simpler just to hoist this to the top, which would still save the potentially unnecessary built-ins check.

src/container-instance.class.mts

freshgum-bubbles · 2023-12-16T02:19:25Z

src/container-instance.class.mts

@@ -1494,35 +1495,6 @@ export class ContainerInstance implements Disposable {
    }
  }

-  private resolveTypeWrapper(wrapper: TypeWrapper): ServiceIdentifier {


The removal of this, even with it being private, should be in the changeset somewhere...

freshgum-bubbles · 2023-12-16T02:19:57Z

src/contrib/lazy-ref/lazy-ref-host.class.mts

+
+  protected readonly container: ContainerInstance;
+  protected readonly constraints: number;
+  protected readonly getID: LazyRefFactory<TIdentifier>;


Would be nice to have some documentation for getID.

freshgum-bubbles · 2023-12-16T02:20:10Z

src/contrib/lazy-ref/lazy-ref-host.class.mts

+
+export type LazyRefFactory<TIdentifier extends ServiceIdentifier> = () => TIdentifier;
+
+export class LazyRefHost<


This needs a sprinkling of TSDoc.

freshgum-bubbles · 2023-12-16T02:20:39Z

src/contrib/lazy-ref/lazy-ref-host.class.mts

+  }
+
+  public createOrNull(): TInstance | null {
+    return this.resolve(ResolutionConstraintFlag.Optional);


Could do with the comment in create, but inverted.

freshgum-bubbles · 2023-12-16T02:22:47Z

.changeset/lucky-insects-fold.md

+
+**This is a breaking change for anyone constructing `TypeWrapper` objects.**
+
+Internal `TypeWrapper` objects have been refactored.


These changesets are a right mess.

freshgum-bubbles · 2023-12-16T02:23:10Z

.changeset/lucky-insects-fold.md

+
+Now, the `eagerType` property is optional, and the `lazyType` property has been removed. This has been changed because the `lazyType` property was part of broader lazy functionality (alongside the `Lazy` function) which was mainly a remnant from the upstream TypeDI project.
+
+Lazy reference functionality has been moved into a contributory package, appropriately named *lazy-ref*.  This package contains a new `LazyRef` function, which allows for holding weak, or "lazy" references to services.


This deserves its own changeset.

freshgum-bubbles · 2023-12-16T02:23:26Z

.changeset/six-seahorses-raise.md

+'@freshgum/typedi': patch
+---
+
+Two new type-wrapper helper interfaces have been added:


Some examples of these might be nice.

freshgum-bubbles

A few more notes to self.

freshgum-bubbles · 2023-12-21T23:20:13Z

src/utils/resolve-to-type-wrapper.util.mts

-   * ? We want to error out as soon as possible when looking up services to inject, however
-   * ? we cannot determine the type at decorator execution when cyclic dependencies are involved
-   * ? because calling the received `() => MyType` function right away would cause a JS error:
-   * ? "Cannot access 'MyType' before initialization", so we need to execute the function in the handler,
-   * ? when the classes are already created. To overcome this, we use a wrapper:
-   * ?  - the lazyType is executed in the handler so we never have a JS error
-   * ?  - the eagerType is checked when decorator is running and an error is raised if an unknown type is encountered
-   */


Was this documentation important? Should it be replaced with something more relevant, which details how type wrappers currently work?

freshgum-bubbles

Few more ideas

Goodbye to a pretty useless function. It was handy for testing extractable type-wrappers, but not for anything else. It has also been removed from the index.

Welcome the newest member to the contrib family: LazyRefHost! This is basically the Lazy function, but... it's actually lazy. It performs very similarly to TransientRefHost, though it operates on lazy references instead of transient ones.

Make the ID property of the DependencyDescriptor interface optional, to support cases wherein the eagerType of a TypeWrapper is not present.

...as opposed to playing double-duty for both TypeWrapper changes and Lazy.

freshgum-bubbles marked this pull request as draft November 29, 2023 16:16

freshgum-bubbles commented Dec 8, 2023

View reviewed changes

freshgum-bubbles commented Dec 16, 2023

View reviewed changes

src/container-instance.class.mts Show resolved Hide resolved

freshgum-bubbles commented Dec 16, 2023

View reviewed changes

freshgum-bubbles commented Dec 21, 2023

View reviewed changes

freshgum-bubbles force-pushed the refactor/replace-lazy-with-lazyref branch from 7a216b8 to 68c1a15 Compare May 21, 2024 10:08

freshgum-bubbles added 14 commits August 1, 2024 19:49

refactor(types): add more members to ContainerInternals

4f82483

refactor(contrib/util): add abstract RefHost class

0f2b902

refactor(functions): remove Lazy

6000327

Goodbye to a pretty useless function. It was handy for testing extractable type-wrappers, but not for anything else. It has also been removed from the index.

refactor(contrib/transient-ref): use RefHost

5b350ee

feat(contrib/lazy-ref): add LazyRefHost

89b3c21

Welcome the newest member to the contrib family: LazyRefHost! This is basically the Lazy function, but... it's actually lazy. It performs very similarly to TransientRefHost, though it operates on lazy references instead of transient ones.

refactor(contrib/util): make RefHost.id an accessor

367092c

fix(contrib/lazy-ref): fix 3AM code

99add54

fix(contrib/lazy-ref): fix Lazy function

5ce2f88

chore(ci): add changeset for LazyRef

230582c

fix(contrib/lazy-ref): remove lazyType

88f8813

refactor(types): move around typewrappers

55db29b

fix(core): remove usage of lazyType

c1602cd

refactor(container): simplify type-wrapper resolution

20e965d

refactor(types): make id optional for factories

d326e25

Make the ID property of the DependencyDescriptor interface optional, to support cases wherein the eagerType of a TypeWrapper is not present.

freshgum-bubbles and others added 3 commits August 1, 2024 19:49

chore(ci): add changesets for TypeWrapper refactor

a9b6bb7

chore(ci): delete unnecessary changeset

25e42bb

refactor(ci): make type-wrapper changeset specifically about them

c8889a9

...as opposed to playing double-duty for both TypeWrapper changes and Lazy.

freshgum-bubbles force-pushed the refactor/replace-lazy-with-lazyref branch from 68c1a15 to c8889a9 Compare August 1, 2024 18:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace Lazy with LazyRef #141

Replace Lazy with LazyRef #141

freshgum-bubbles commented Nov 28, 2023

changeset-bot bot commented Nov 28, 2023 •

edited

Loading

freshgum-bubbles left a comment

freshgum-bubbles Dec 8, 2023 •

edited

Loading

freshgum-bubbles Dec 8, 2023

freshgum-bubbles Dec 8, 2023

freshgum-bubbles Dec 16, 2023

freshgum-bubbles Dec 16, 2023

freshgum-bubbles Dec 16, 2023

freshgum-bubbles Dec 16, 2023

freshgum-bubbles Dec 16, 2023

freshgum-bubbles Dec 16, 2023

freshgum-bubbles Dec 16, 2023

freshgum-bubbles Dec 16, 2023

freshgum-bubbles left a comment

freshgum-bubbles Dec 21, 2023

freshgum-bubbles left a comment


		export type LazyRefFactory<TIdentifier extends ServiceIdentifier> = () => TIdentifier;

		export class LazyRefHost<


		This is a breaking change for anyone constructing `TypeWrapper` objects.

		Internal `TypeWrapper` objects have been refactored.


		Now, the `eagerType` property is optional, and the `lazyType` property has been removed. This has been changed because the `lazyType` property was part of broader lazy functionality (alongside the `Lazy` function) which was mainly a remnant from the upstream TypeDI project.

		Lazy reference functionality has been moved into a contributory package, appropriately named lazy-ref. This package contains a new `LazyRef` function, which allows for holding weak, or "lazy" references to services.

Replace Lazy with LazyRef #141

Are you sure you want to change the base?

Replace Lazy with LazyRef #141

Conversation

freshgum-bubbles commented Nov 28, 2023

changeset-bot bot commented Nov 28, 2023 • edited Loading

🦋 Changeset detected

freshgum-bubbles left a comment

Choose a reason for hiding this comment

freshgum-bubbles Dec 8, 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

freshgum-bubbles left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freshgum-bubbles left a comment

Choose a reason for hiding this comment

changeset-bot bot commented Nov 28, 2023 •

edited

Loading

freshgum-bubbles Dec 8, 2023 •

edited

Loading