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: make Observable class internal #690

Open
2 of 3 tasks
zccz14 opened this issue Jul 11, 2024 · 3 comments · Fixed by #687 or #691
Open
2 of 3 tasks

feat: make Observable class internal #690

zccz14 opened this issue Jul 11, 2024 · 3 comments · Fixed by #687 or #691
Labels
enhancement New feature or request

Comments

@zccz14
Copy link
Contributor

zccz14 commented Jul 11, 2024

Describe the feature

Check all libraries's exporting API:

  • Remove all Observable<T> and replace them with AsyncIterable<T>
  • Remove all Subject<T> and replace them with NativeSubject<T> = AsyncIterable<T> | AsyncIterator<T, void, T>.
  • Remove all ObservableInput<T> and replace them with AsyncIterable<T> | PromiseLike<T> | ArrayLike<T> | Iterable<T>

Hint: All Observable<T> can be transformed to AsyncIterable<T>, and back to Observable<T>.

Why do you need this feature?

It might cause compatibility issues.

Confusing when someone is new to development.

Additional context

Observable of rxjs is useful for defining a stream interface.
But, the issue is that when we export an interface with Observable, it will bind the version of rxjs. (because Observable is a class rather than an interface)
We met a compiling error: Observable cannot be assigned to another Observable from a different version of rxjs.

The solutions are:

  1. Keep all related rxjs in the same version. (Hard to ensure when code dispersed in multiple repos)
  2. Make rxjs as a peer-dependency. (Peer dependency has many unresolved issues)
  3. Ensure all Observables are used inside the package, and never export them as Param and Return types.

To introduce the short rule: Never export a class type

In JavaScript & node modules cache & npm:

  1. Class is a named type, not a structured type (duck type), with a more strict compatible issue.
  2. Node.js distinct JS files by its path.
  3. You can install different versions of one node package, but they will be installed in different paths.

So, exporting a class type might cause compatibility issues.

@zccz14 zccz14 added the enhancement New feature or request label Jul 11, 2024
@zccz14
Copy link
Contributor Author

zccz14 commented Jul 11, 2024

#687 introduces an util to get AsyncIterable from Observable.
from in package rxjs can be used to get Observable from AsyncIterable.

@zccz14
Copy link
Contributor Author

zccz14 commented Jul 11, 2024

I think Subject<T> is equal to the read-side AsyncIterable<T> and the write-side AsyncIterator<T, void, T>.

Define a type NativeSubject<T> and transform between native subject and rxjs subject:

/**
 * NativeSubject is native version of rx's Subject, which can be used in async generator.
 *
 * @public
 */
export type NativeSubject<T> = AsyncIterable<T> & AsyncIterator<T, void, T>;

/**
 * convert a rx's Subject to a NativeSubject.
 *
 * @param subject$ - the rx's Subject to convert
 * @returns a NativeSubject
 *
 * @public
 */
export const subjectToNativeSubject = <T>(subject$: Subject<T>): NativeSubject<T> => ({
  ...observableToAsyncIterable(subject$),
  next: (value: T) => {
    subject$.next(value);
    return Promise.resolve({ value, done: false });
  },
  return: () => {
    subject$.complete();
    return Promise.resolve({ value: undefined, done: true });
  },
  throw: (e: any) => {
    subject$.error(e);
    return Promise.reject(e);
  },
});

/**
 * convert a NativeSubject to a rx's Subject.
 *
 * @param source - the NativeSubject to convert
 * @returns a rx's Subject
 *
 * @public
 */
export const nativeSubjectToSubject = <T>(source: NativeSubject<T>): Subject<T> => {
  const subject$ = new Subject<T>();
  subject$.subscribe({
    next(v) {
      source.next(v);
    },
    complete() {
      source.return?.();
    },
    error(e) {
      source.throw?.(e);
    },
  });
  return subject$;
};

@zccz14
Copy link
Contributor Author

zccz14 commented Jul 12, 2024

#693 implemented

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
1 participant