Proposal: Move csv/yaml/toml/json/jsonc/front_matter to top level

[kt3k]

These 6 modules in std/encoding are relatively complex and look worth top level modules.

Also there are claims from contributors that the current structures of these modules are unreasonable/inconsistent (#2538). For example, csv related APIs are exported from encoding/csv.ts and encoding/csv/stream.ts, and because of this structure encoding/csv isn't in the correct form of module. (There's another proposal to move encoding/csv.ts to encoding/csv/mod.ts, but by doing this these APIs will become less discoverable)

I propose we should move these like the below:

• encoding/csv.ts -> csv/mod.ts
• encoding/yaml.ts -> yaml/mod.ts
• encoding/toml.ts -> toml/mod.ts
• encoding/json/*.ts -> json/mod.ts
• encoding/jsonc.ts -> jsonc/mod.ts
• encoding/front_matter.ts -> front_matter/mod.ts

related #2538

[lino-levan]

I wasn't really following the discussion in many of the related issues. Three quick notes:

• This is obviously a breaking change, are there any other changes we can make at the same time to avoid a double breaking change on these? (I think @timreichen has at least one proposal on csv parsing)
• Is the discoverability an issue with the directory structure or an issue with dotland? Imo, it feels like a dotland issue.
• This is just me spitballing, should we consider merging the json and jsonc folders along with this change?

[iuioiua]

• This is obviously a breaking change, are there any other changes we can make at the same time to avoid a double breaking change on these? (I think @timreichen has at least one proposal on csv parsing)

The move towards single-export files, which @timreichen aimed to do in his PRs as part of #2936.

• Is the discoverability an issue with the directory structure or an issue with dotland? Imo, it feels like a dotland issue.

Directory structure - perhaps having the various formats under the one directory was once okay, but now the implementations seemed to have outgrown std/encoding, and each deserves to be a module on its own.

• This is just me spitballing, should we consider merging the json and jsonc folders along with this change?

I don't think we should. Even though they're both JSON-related, they don't have much overlap.

[lino-levan]

(I think @timreichen has at least one proposal on csv parsing)

I was referring to #2660 in regards to this. We should probably make a decision on this as we are moving the files.

[pting-me]

I agree with @lino-levan as it seems like a dotland issue.

Take a look at dotland. Say for example I'm looking for a JSON encoder. It's not particularly easy for me to look at the std library and see where it is. Searching isn't particularly easy either. Really the best way for me to find it is by searching through the GitHub source code.

Conversely, look at Go stl. While their directory search doesn't seem to work for me, but at least I can "expand all" and search for it.

[kt3k]

I updated the 2nd paragraph of the issue description to clarify the purpose. Another important motivation is to resolve the current unreasonable structure of encoding/csv.ts + encoding/csv/stream.ts

The discoverability in the web site can be solved by dotland repo, but there's also discoverability issue from IDE's import path completion. If we move these to the top level, they appear immediately after typing https://deno.land/std[@version]/

[pting-me]

That's very interesting.

I just realized that this is not a concern in Node or Golang.

In Node, Intellisense knows what to do because of node_modules.

In Golang, it looks like autocomplete within the import statements is not a feature they had to begin with... Intellisense kicks in when writing code within the body, and offers up whatever is in the std library, in addition to whatever has been imported.

I wonder if deno_std should be a first class citizen and cached by default. That way we can get the same sort of autoimport behavior in Node and Golang, and not have to make guesses on the deno_std directory structure.

[lino-levan]

The discoverability in the web site can be solved by dotland repo, but there's also discoverability issue from IDE's import path completion

I understand the concern here but I feel like this doesn't really affect a large group of people. If you've never touched deno before, you wouldn't know the structure of std and thus would not rely on ide autocomplete and go on the web to find what you are trying to do. If you are somewhat experienced in deno, you would know the location of the encoding (or would be able to figure it out pretty easily).

To clarify: I'm not necessarily against moving the proposed modules to the top level, but I think:

• The arguments to do this aren't super strong
• Pretty big breaking change (for imo little gain)
• Even with this change modules in std will still not have great discoverability for beginners which I assume is the main reason for considering this change

[timreichen]

• Pretty big breaking change (for imo little gain)

I think it is a very big gain, because we will move towards a consistent file structure in std (no separate dirs and files for the same module as it is now).

• Even with this change modules in std will still not have great discoverability for beginners which I assume is the main reason for considering this change

I think the main reason for this change is consistency in the std file structure. That should be a good enough reason imo.

[lino-levan]

because we will move towards a consistent file structure in std

Ideologically, I agree. We should have more than just ideological arguments for why we want to do things however (in my opinion).

Is this a practical change? Is this issue a symptom of something else (outside of the file structure)? I think these are the questions that need discussion.

[iuioiua]

The reasons for the change are to improve the file structure. A way to look at it is, being consistent with the rest of the repo, we would have CSV in a top-level folder. What valid reason would we have to make an exception and make it a submodule under encoding? I think these submodules have outgrown the encoding folder and deserve their own top-level folders. The technical reasons for doing this don't have as much weight, for me at least.

Should the varint folder also be included in the move, @kt3k?

[kt3k]

I personally don't think varint is worth the top level module as it's not that common as csv, yaml, etc. I now feel the usage of the wasm for varint was a kind of mistake regarding the complexity of the source code. Maybe let's replace the implementation with typescript if we really don't like encoding/varint submodule (Maybe we can port this implementation https://github.com/chrisdickinson/varint

[kt3k]

@lino-levan

Is this a practical change? Is this issue a symptom of something else (outside of the file structure)? I think these are the questions that need discussion.

Practical aspect of this proposal (i.e. better discoverability) is just supporting points in my view. We usually don't do breaking changes just to make such slight discoverability improvements. The root motivation of this proposal is the resolution of the inconsistency in file/module structures.

[iuioiua]

I personally don't think varint is worth the top level module as it's not that common as csv, yaml, etc. I now feel the usage of the wasm for varint was a kind of mistake regarding the complexity of the source code. Maybe let's replace the implementation with typescript if we really don't like encoding/varint submodule (Maybe we can port this implementation https://github.com/chrisdickinson/varint

Sounds like fun. I'll do it.

[iuioiua]

encoding/varint was implemented in #2265. According to the Discord conversation, a pure TypeScript implementation would obviously be slower, but I'm not sure whether performance is crucial in this instance. I think it's worth it for a simpler implementation. I think we should vendor https://deno.land/x/[email protected]. @kt3k, does that sound good?

CC @MierenManz for any other opinions, as you're the original implementer of encoding/varint.

[MierenManz]

The way it's done in x/varint is slower for both u32 and u64 but for u32 it is possible to have it as fast as wasm. Just that u64 will be slow due to bigints not being that fast.

I have a JavaScript version somewhere for u32 decoding and encoding which should be near wasm performance. For u64 it's just not possible to get the same performance I think

[iuioiua]

Are there any other submodules that these changes could apply to?

[kt3k]

I think all are done now. Thanks Asher for these works!

[not-my-profile]

I just found this issue ... I really don't think that it makes sense to abandon structure just to cater to the poor search implementation of tooling (the deno.land website and the language server) when these shortcomings could very well be addressed without resorting to having all modules at the top-level. Please join #3462 for a discussion about that.