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: move closer towards single-export files #2936

Closed
8 tasks done
iuioiua opened this issue Nov 24, 2022 · 8 comments
Closed
8 tasks done

suggestion: move closer towards single-export files #2936

iuioiua opened this issue Nov 24, 2022 · 8 comments

Comments

@iuioiua
Copy link
Collaborator

iuioiua commented Nov 24, 2022

The std/collections, std/async and std/fs modules are great. They're structured so that each file essentially has one export. This makes the purpose of each file clear. They also have the added benefit of allowing the developer to keep dependencies lean but provide an alternative of importing multiple functions via their respective mod.ts files if needed.

On the other hand, other modules like std/streams and std/io do not follow this "single-export-per-file" rule. Instead, they're further sub-categorised, which can sometimes be unclear, requiring the developer to read the file before understanding its purpose (io/files.ts comes to mind). Furthermore, multiple functions are grouped together. For example, if I want to use writeAll() from streams/conversion.ts, I also pull in 14 other functions/classes/interfaces.

For these reasons, I suggest we:

  1. Move functions to their own files, where appropriate, along with testing counterparts. Async and sync siblings can remain in the same file.
  2. Ensure a mod.ts file exists within each module that exports all functions/interfaces/classes within that module.
  3. Point internal imports to their new locations.
  4. Deprecate the old locations with a clear explanation and ample time until their removals.

This needs to be done for the following modules:

  • tar
  • bytes
  • crypto
  • datetime
  • io
  • streams
  • media_types
  • encoding
@lino-levan
Copy link
Contributor

I'd like to see what this would look like, but I'm a little bit skeptical as this seems like it will be a very noisy change. Would be a big fan of seeing a draft PR though.

@lino-levan
Copy link
Contributor

I have been thinking about this one recently. I don't think flags really applies here given that flags literally only has one (non-type) export.

I also have similar feelings for testing, though I'd be interested in hearing a counterargument on that one. It feels like single-export files for testing would be a overkill and really messy given that many of the functions in it are literally only a few lines of code.

@iuioiua
Copy link
Collaborator Author

iuioiua commented Dec 11, 2022

I don't think flags really applies here given that flags literally only has one (non-type) export.

Agreed. I've removed it from the list.

I also have similar feelings for testing, though I'd be interested in hearing a counterargument on that one. It feels like single-export files for testing would be a overkill and really messy given that many of the functions in it are literally only a few lines of code.

The counterarguments would be those mentioned in the initial comment. I'm fine with either option. Let's see what others think.

@iuioiua
Copy link
Collaborator Author

iuioiua commented Dec 27, 2022

Are there any benefits to be had by restructuring testing?

@lino-levan
Copy link
Contributor

I will restate my previous opinion that we should keep testing the way it currently is. Curious to hear what the broader community thinks.

@iuioiua
Copy link
Collaborator Author

iuioiua commented Jan 6, 2023

@kt3k, WDYT?

@kt3k
Copy link
Member

kt3k commented Jan 6, 2023

I'd prefer to keep the current structure of testing by similar reasons @lino-levan gives.

Also I think people don't care much about load footprint of test utilities than other non-testing utility. Even if we provide testing/assert_equals.ts, I guess people would prefer to keep importing from testing/asserts.ts (as it's more memorable, requires less typing)

@iuioiua
Copy link
Collaborator Author

iuioiua commented Mar 18, 2023

These are all now complete.

@iuioiua iuioiua closed this as completed Mar 18, 2023
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

No branches or pull requests

3 participants