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

Remove "standalone" resource method from Deno namespace #12107

Closed
kitsonk opened this issue Sep 16, 2021 · 10 comments
Closed

Remove "standalone" resource method from Deno namespace #12107

kitsonk opened this issue Sep 16, 2021 · 10 comments
Assignees
Labels
breaking change a change or feature that breaks existing semantics feat new feature (which has been agreed to/accepted)
Milestone

Comments

@kitsonk
Copy link
Contributor

kitsonk commented Sep 16, 2021

Methods like Deno.read() and Deno.write() are effectively redundant as there are other better objects to manage the resources (like Deno.File) and having to deal explicitly with the rid which is just a number can be confusing and error prone to users.

@kitsonk kitsonk added feat new feature (which has been agreed to/accepted) maybe 2.0 labels Sep 16, 2021
@kitsonk kitsonk mentioned this issue Sep 17, 2021
17 tasks
@kitsonk kitsonk added this to the 2.0.0 milestone Sep 17, 2021
@caspervonb
Copy link
Contributor

Methods like Deno.read() and Deno.write() are effectively redundant as there are other better objects to manage the resources (like Deno.File) and having to deal explicitly with the rid which is just a number can be confusing and error prone to users.

I'm in favour but note that that this will completely break std/wasi, that's okay but I'll have to rewrite it.

@kitsonk kitsonk added the breaking change a change or feature that breaks existing semantics label Sep 17, 2021
@bartlomieju
Copy link
Member

Deno.seek() and Deno.seekSync() should be removed as well.

@bartlomieju
Copy link
Member

More methods that should be moved to Deno.File class:

  • Deno.fsyncSync
  • Deno.fsync
  • Deno.fdatasyncSync
  • Data.fdatasync
  • Deno.flock
  • Deno.flockSync
  • Deno.funlock
  • Deno.funlockSync

CC @caspervonb

@caspervonb
Copy link
Contributor

More methods that should be moved to Deno.File class:

  • Deno.fsyncSync
  • Deno.fsync
  • Deno.fdatasyncSync
  • Data.fdatasync
  • Deno.flock
  • Deno.flockSync
  • Deno.funlock
  • Deno.funlockSync

CC @caspervonb

SGTM, patches coming right up.

@wojpawlik
Copy link

Should Deno.close be deprecated in favor of Deno.File::close?

@bartlomieju
Copy link
Member

Should Deno.close be deprecated in favor of Deno.File::close?

It's still debated, but most likely it won't be deprecated.

@caspervonb
Copy link
Contributor

caspervonb commented Sep 19, 2021

Should Deno.close be deprecated in favor of Deno.File::close?

It's still debated, but most likely it won't be deprecated.

I think we should, in favour of using explicit resource management (https://github.com/tc39/proposal-explicit-resource-management stage 2) and finalisers.

Isn't the point of removing these to get rid of isomorphic resources internally?

@bartlomieju
Copy link
Member

Should Deno.close be deprecated in favor of Deno.File::close?

It's still debated, but most likely it won't be deprecated.

I think we should, in favour of using explicit resource management (https://github.com/tc39/proposal-explicit-resource-management stage 2) and finalisers.

Isn't the point of removing these to get rid of isomorphic resources internally?

It's a good point and that proposal looks interesting. Ideally we should eyeball adding automatic cleanup for all resources using FinalizationRegistry. I'm not sure though, if we could deliver on that for 2.0.

@piscisaureus
Copy link
Member

I don't see the point of removing these methods at this time - it just breaks user code unnecessarily.
We should have a discussion on how to proceed with automatic resource cleanup first.

@bartlomieju
Copy link
Member

This is now done as of #25213 and Deno 2.0.0-rc.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change a change or feature that breaks existing semantics feat new feature (which has been agreed to/accepted)
Projects
None yet
Development

No branches or pull requests

5 participants