Naming of exports

[marijnh]

The behavior work is creating a lot of related types, functions, and constants that extending code will need access to—things to define behaviors and extensions, pre-defined behaviors, the types of those behaviors and extensions, etc.

I'm opening this issue as a place to organize my thoughts and invite discussion.

Namespacing

So far, I've been leaning towards using as flat as possible a namespace within each package—everything goes into the top exports, except for the occasional static method that reads better when tacked onto a class.

I'm a bit concerned about our namespaces (in the state and view packages), and thus our docs, becoming an unclear mess when they are littered with too many individual exports. Also, names in such a namespace tend to become long (defineViewBehavior).

One approach to avoiding this would be to introduce namespaces inside the packages, within classes or, if the concept isn't a class, in a TypeScript namespace named after the relevant type. So you'd get ViewBehavior.define instead. The disadvantages of that are:

• Such names can no longer be imported conveniently with an import statement, and thus if you need such values or types often in a piece of code, you'll have to either define it locally with a const or type declaration, or repeat ViewBehavior. a lot.

• Rollup and other bundlers probably won't collapse such bindings to local identifiers, so they leave behind an actual object access (more bytes) in the bundled code.

• Depending on how clever a tree shaker is, it might not be able to tree-shake the junk that TypeScript compiles namespaces to (iifes assigning to an object they get as argument).

The other approach would be to make sure our doc generating tool supports organizing bindings under sections (as builddocs does for the ProseMirror sources via the src/README.md mechanism), and hope that's enough to keep the docs clear.

Contextual naming

Instead of calling something ViewBehavior or even EditorView, we could create names in the context of the package that they belong to (Behavior / View) to make them less verbose. The main problem with this is name clashes (whenever you need both view behavior and state behavior, you'll have to import Behavior as ViewBehavior etc to disambiguate, and names like Selection clash with definitions from the dom types, which is even more annoying).

The current situation is a bit messy (I'm EditorView because View seems too minimal, but I am using ViewBehavior because EditorViewBehavior is just too much).

So naming remains hard. I guess we can continue just winging it, and figuring out the least problematic approach case-by-base, but it'd be nice if we could agree on some kind of basic stylistic choices. Thoughts?

[adrianheine]

I think you always prefix core concepts with Editor and drop that prefix in sub-concepts (like you described). I see the advantages, but it leads to inconsistent names and I always have to scan for the matching part to see whether some concepts belong together.

For me, View and State would be acceptable terms within our code base; users could always alias-import (and probably would). See also how this is handled in rust world (with better language support, though): 1, 2.

[marijnh]

I like that general idea.

In pre-destructuring CommonJS times, people were kind of guided in that direction since doing let editor = require("codemirror-view"); editor.View was the easiest way to import stuff, but with ES6 import * as editor from "codemirror-view" is kind of the awkward, uncommon way, and by doing the straightforward thing (import {View} from "codemirror-view") you will end up with View in your local scope. I'm a big fan of straightforward code, and I feel this would kind of be a nudge in the wrong direction for users (View is a really poor local name, in most contexts).

Still, that might be less bad than the inconsistent EditorView/ViewExtension issue.

One catch is that renaming EditorSelection to Selection conflicts with the browser built-in type Selection, which you never interact with directly, but which you often need as a type when writing TypeScript. Should we just also expect people to as-import that? It's not a big deal, but will cause a lot of user annoyance as people run into that issue for the first time.

[marijnh]

(Then again, typical client code will probably not need to pass around DOM selection objects very often.)

[marijnh]

Oh, and isn't prosemirror-view.View / prosemirror-view.ViewUpdate still technically stuttering? With the first, well, we have to give it some name (at one point Rust allowed submodules and values to share a name, I'm not sure if that's still the case, but it was cute). But with the second, would Update be preferred? Feels a bit too cryptic.

[adrianheine]

Hm, I think I'm not happy with this either. I thought we wouldn't have to qualify internally, but that doesn't work. If for example a history module has a field, and that's called (history.)Field, we have to import state.Field as StateField.

[marijnh]

At this point I think no one really likes intra-module nesting very much, so the resolution of the first point seems to be 'don't nest'.

The second problem --- how much context to tack onto the front of the names --- we seem to agree that the current solution (adding just enough context to avoid clashes—EditorView but ViewExtension) is too unpredictable and messy, but don't really have a solution yet. Always fully qualifying would get very verbose (prosemirror-view.EditorViewExtension) and repetitive. Dropping context (View, Extension) is annoying because everyone will have to import qualified to avoid name clashes or locally meaningless names.

For some things, we could try to find less generic (yet short) names to both make the fully-contextualized names less obnoxious and to avoid name clashes. I.e. if (not saying that's a good idea) we used CM instead of EditorView, CMExtension and CMUpdate would be both quick to type and unlikely to clash. Unfortunately, they'd still be hideous, so finding a good word there is an open problem.

[curran]

This reminds me of "the great namespace flattening" that happened in D3.js at v4.

However, there is one unavoidable consequence of adopting ES6 modules: every symbol in D3 4.0 now shares a flat namespace rather than the nested one of D3 3.x. For example, d3.scale.linear is now d3.scaleLinear, and d3.layout.treemap is now d3.treemap. -- Changes in D3 4.0

The context comes after the names, so alphabetical ordering results in logical tree ordering.

For example, EditorViewExtensionProsemirror instead of ProsemirrorEditorViewExtension.

Related reading that might be interesting (by Mike Bostock on D3 v4): What Makes Software Good?.

[curran]

FWIW these are the export names I'm working with at the moment:

import {
  EditorState,                 
  EditorView,                  
  keymap,
  history,                     
  redo,
  redoSelection,               
  undo,                        
  undoSelection,
  lineNumbers,                 
  baseKeymap,
  indentSelection,             
  legacyMode,
  matchBrackets,               
  javascript,
  specialChars,                
  multipleSelections           
} from '@datavis-tech/codemirror-6-prerelease';

Defined in https://github.com/datavis-tech/codemirror-6-prerelease/blob/master/codemirror.ts

IMO clashes are not a problem as you can always alias an import, e.g.

import { EditorState as CMEditorState } from ...

[marijnh]

I think I've just accepted that some inconsistencies (EditorView but ViewPlugin, not EditorViewPlugin) are unavoidable if we don't want the interface to feel like it was written by computers, for computers. Also, nesting just doesn't work well with TypeScript, so that's out.

Closing this as "too bad we couldn't find a grand unified consistent naming scheme, but we get by pretty well without".