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

Suggestion: Consider adding global container object for disposables #41

Closed
rbuckton opened this issue Nov 10, 2019 · 3 comments · Fixed by #56
Closed

Suggestion: Consider adding global container object for disposables #41

rbuckton opened this issue Nov 10, 2019 · 3 comments · Fixed by #56

Comments

@rbuckton
Copy link
Collaborator

rbuckton commented Nov 10, 2019

I'm considering adding something like the following to the global scope:

class Disposable {
  static from(disposables);
  constructor(onDispose);
  [Symbol.dispose]();
}

class AsyncDisposable {
  static from(disposables);
  constructor(onAsyncDispose);
  [Symbol.asyncDispose]();
}

These could act as containers to aggregate disposables, guaranteeing that every disposable resource in the container is disposed when the respective disposal method is called. If any disposable in the container throws an error, they would be collected and an AggregateError would be thrown at the end.

This is a common pattern in several languages that support this feature, and is often used in such things as VS Code extensions:

// vscode extension example...
import { Disposable } from "vscode";
class LanguageService {
  constructor() {
    // NOTE: vscode uses `from` instead of `of`
    this._disposable = Disposable.from(
      this._hoverProvider = new MyHoverProvider(),
      this._referenceProvider = new MyReferenceProvider(),
      this._decorationProvider = new MyDecorationProvider()
    );
  }
  dispose() {
    this._disposable.dispose(); // disposes of all three
  }
  ...
}

let languageService;
export function activate(context) {
  // NOTE: subscriptions are disposed when extension is deactivated
  context.subscriptions.push(languageService = new LanguageService()); 
}

In addition, if we opt to include a using value statement as mentioned here, this would allow us to emulate Go's defer statement:

const file = openFile();
using value new Disposable(() => file.close());
// in Go:
// defer file.close()

While Go's syntax is more succinct, it doesn't enforce a standard pattern for resource management, which is one of the goals of this proposal.

This would also allow us to support an "async" defer-like statement:

{
  await updateTitle("Starting task...");
  await startSpinner();

  // setup UI updates for when block exits...
  using value new AsyncDisposable(async () => {
    await updateTitle("Task exiting...");
    await stopSpinner();
  });

  // do work...
}
@acutmore
Copy link

acutmore commented Jan 1, 2021

I'm sure RxJs would love to see a standardised Disposable (they renamed it to Subscription in v5 but same thing)

https://github.com/ReactiveX/rxjs/blob/8c13e0a3dbd88cc473677cd41c9f09cc26a3d947/src/internal/Subscription.ts

EDIT: In the RxJs version you can add more cleanups post constructor. Which I have found very useful. So would love to see a similar thing here.

// in classes
class SomeService {
  disposable = new Disposable();

  constructor() {
      setupA();
      setupB();
  }
  
  setupA() {
     this.disposable.add(new A());
  }

  setupB() {
     this.disposable.add(new B());
  }
  
  dispose() {
    this.disposable.dispose();
  }
  ...
}
// in React.js

useEffect() => {
  let disposable = new Disposable();
  
  disposable.add(stream1
    .map(...)
    .filter(...)
    .subscribe(...)
  );

  let x = workOutSomethingMidway();
  
  disposable.add(stream2
    .flatMap(...)
    .delay(...)
    .subscribe(...)
  );  

  return () => disposable.dispose();
}, [deps]);

@rbuckton
Copy link
Collaborator Author

rbuckton commented Oct 6, 2021

I'm not sure I want to introduce .add (or .remove) currently, as that could result in unexpected changes to disposal order. You can do the above in user code with the current Disposable proposal, however:

// in classes
class SomeService {
  disposables = [];

  constructor() {
      setupA();
      setupB();
  }
  
  setupA() {
     this.disposables.push(new A());
  }

  setupB() {
     this.disposables.push(new B());
  }
  
  dispose() {
    Disposable.from(this.disposables)[Symbol.dispose]();
  }
  ...
}
// in React.js

useEffect() => {
  let disposables = [];
  
  disposables.push(stream1
    .map(...)
    .filter(...)
    .subscribe(...)
  );

  let x = workOutSomethingMidway();
  
  disposables.push(stream2
    .flatMap(...)
    .delay(...)
    .subscribe(...)
  );  

  return () => Disposable.from(disposables)[Symbol.dispose]();
}, [deps]);

@acutmore
Copy link

acutmore commented Oct 7, 2021

Agreed that userland can offer these types of helpers. Mutable disposables with .add and .remove do complicate things, the space for different semantics grows and picking which one is ‘best’ gets more and more subjective.

For others reading this thread but not this one. If they want a bit more safety in their useEffect hook:

useEffect() => {
  let cleanupTransferred = false;
  let disposables = [];
  using const (new Disposable(() => {
   if (cleanupTransferred) return;
   using const (Disposable.from(disposables))
  }));
  
  disposables.push(stream1
    .map(...)
    .filter(...)
    .subscribe(...)
  );

  let x = workOutSomethingMidway();
  
  disposables.push(stream2
    .flatMap(...)
    .delay(...)
    .subscribe(...)
  );  

  cleanupTransferred = true;
  return () => Disposable.from(disposables)[Symbol.dispose]();
}, [deps]);

Which could be wrapped up as a userland helper and look like:

useEffect() => {
  using const subs = new ResourseHelper();
  
  subs.hold(stream1
    .map(...)
    .filter(...)
    .subscribe(...)
  );

  let x = workOutSomethingMidway();
  
  subs.hold(stream2
    .flatMap(...)
    .delay(...)
    .subscribe(...)
  );  

  return subs.transferToFunction();
}, [deps]);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants