Deprecating antlr4ts import with paths

[BurtHarris]

Breaking Change in Imports Planned

Imports of antlr4ts runtime classes will need to change. In the future, this should simplify matters so that imports from the antlr4ts public API can be combined and simplified, to something like this:

import { Parser, ParseTree, ParseTreeListener, TerminalNode, ErrorNode } from 'antlr4ts'

Background

In the current alpha-quality prerelease, it's sometimes been necessary to use multi-part paths when writing import statements, and the exact nature of those paths has been different for code inside this repository, vs outside this project's repository. For example, as of 0.4.0, actual client code outside this repository might use

import { LexerATNSimulator } from 'antlr4ts/atn/LexerATNSimulator';

... to get access to a relatively obscure part of the runtime, but code inside this repository needs to use relative references like

import { BitSet } from "../src/misc/BitSet";

In my experience, neither of these is desirable, as they introduce undesirable coupling, increasing the interdependence between modules. Moving forward, I plan on eliminating this by supporting import statements using a façade which decouples imports of antlr4ts types from the actual location of the file in the source tree. So moving forward we want antlr4ts imports to be more compact and expose less detail of the source tree's organization, allowing the import of a number of public ANTLR related declarations in a single statement, as illustrated at the top.

Narrowing the 'public' API

In adopting semantic versioning for version 1.0, we'll want to reduce the surface-area of antlr4ts runtime somewhat. Such narrowing of the API is necessarily a breaking change, so I want to get this done fairly quickly.

For example, we currently have stub implementations of decorator classes like @Overload that anyone can use (despite the fact it does nothing) by importing from antlr4ts/Decorators. Decorators are still subject to change by the Ecma TC39 working group, and so these decorators shouldn't be considered part of the antlr4ts public API.

A pattern for non-public access to internal declarations

There may be cases where code (particularly unit tests) needs access to non-public APIs. While in JavaScript its impossible to avoid this possibility, its really not a good idea and so to emphasize that we'll move to a syntax where such internal access requires a change to the import as well. For example, if we determine that class MurmurHash doesn't need to be part of the public antlr4ts API, you will may still import it using something like this:

import { MurmurHash } from 'antlr4ts/internal';

The fact that this possible and can be made type-safe doesn't make it a good idea. Any declaration in the antlr4ts/internal set are subject to change on minor versions, so code dependent on this kind of API should lock-down its dependency to specific versions of the runtime in their package.json file.

Two-phased deprecation

This will eventually be a breaking change, which doesn't bother me much as we're still in alpha release with a 0 major version (which means that semantic versioning doesn't really yet apply.) However, its still probably wise to take a two-phase approach to this, first deprecating the old path-style imports in a non-breaking change that adds support for the new style, and a second change (before or part of 1.0) which completes the transition. Implementing this has proved a bit challenging, but I think I've now got all the parts needed to do so.

@sharwell, @mike-lischke, @fdeitelhoff you input on this is invited.

@fdeitelhoff if you've got some cycles I'd like to work with you to QA this change (part of a larger gulp-build change I'm queueing up) before attempting to merge it. Are you up for this?

[sharwell]

It's already (supposed to be) possible to do this in client code:

import { ParseTree, ParseTreeListener, TerminalNode, ErrorNode } from 'antlr4ts/tree';

I don't understand the need/desire to flatten the namespaces further.

Within the repository, being specific with imports helps prevent problems we've observed with circular references. I do not anticipate making a change to how we import items within the antlr4ts code base itself.

[BurtHarris]

In short it's really a matter of reduced coupling, both of source files and generated file location. This has been a pain in the ass for me working on build scripts etc.. E.g. the current approach leads to a root directory of the package having transpiled files in it, while the general practice for javascript packages seems to be to have a /lib (or similar) directory, but in order to do that we'd need to change the above to import from antlr4ts/lib/tree, itself a breaking change. I want to make the breaking change go in the direction of decreased coupling rather than increasing it. Otherwise we need to add the lib to external imports, and src to internal imports.

[sharwell]

What benefit would we have by moving the transpiled files into a lib folder? It seems like the result would be the same as what we have now, except users would have to add /lib to their import lines.

[BurtHarris]

Yes, but I really don't want things like import {Overload} from 'antlr4ts/Decorators' to continue to work. Files like that should be completely private, but that's pretty much impossible in JavaScript. By introducing a lib folder, way someone would need to change their code to antlr4ts/lib/Decorators, and that's something I think we document as an anti-pattern.

[BurtHarris]

P.S. Moving the transpiled files into lib would be the second-part of the two-phase deprecation. We first need to give them an simple "approved" way to import public API, and that simplified one should flatten the namespace for simplicity.

[sharwell]

@BurtHarris The first part is covered by importing only from folders (antlr4ts, antlr4ts/tree, etc., but not antlr4ts/Decorators). Non-public members were left out of the index files.

[BurtHarris]

I really want it to be drop-dead simple to check that antlr4ts imports are in the recommended format, which means from 'antlr4ts' should work for anything public.

@sharwell, We both understand the what you mean by importing from files vs folders, but that doesn't mean its an obvious distinction, or easily checkable. After all, importing from folder really means something like "importing from a file named index.ts (or .js) in a folder, except for a package-level import where it could mean something different depending on what's in package.json".

For example, the tool generated code is importing from individual files, including Decorators. Naturally, some projects have checked-in the generated code and developers have had to hand edit it due to problems in prerelease. It seems to like we should not break that checked-in code in the next minor pre-release, but announce that we will make it fail by version 1.0, so they need to update it, and that's the point behind my reasoning on this issue.

The argument extends to unnecessary distinctions between class and interface, like Vocabulary vs. VocabularyImpl in the generated code, etc. In my mind we should also make internal support classes like ATNDeserializer and Utils; they really shouldn't need to be public.

[sharwell]

@BurtHarris I understand the motivation, but one thing to remember is the original code was never particularly well designed on this particular front. For example, there are several parts of the Java code which expose mutable fields as part of the public API (almost universally seen as a bad practice). I'm not trying to use the TypeScript target to fix these specific issues.

I believe the current layout represents a good balance between usability and separation of packages to match the original targets.

[BurtHarris]

Sure I remember. The original code was in Java, a language that seems to go to unusual lengths enforce a file == class mantra. I believe that model is counterproductive in less class-centric languages. The use of Java was one of the main reasons I never got active in ANTLR prior to this project: Java grates on me. Its that sort of grating feeling that the compromise Java's API model gives me that I want to address now as part of ts-flavor, not issues of mutable fields (which really can't be prevented in JavaScript/TypeScript class-like model.)

One import coding style I do like is import * as antlr from 'antlr4ts'. Then when I want to refer to an ANTLR api, I do it like antlr.ParseTreeListener. No need to go back to the import section to add another name if I haven't used ParseTreeListener before... But as long as we try to keep the directory-based hierarchy in imports, that's not going to work get the full public API.

The TypeScript imports will never match up to the Java version, so evolving them further really isn't a big change. So I'm surprised to get so much push back about going all the way to the from 'antlr4ts' model.

[sharwell]

I'm not necessarily opposed to moving some of the types up for higher visibility, but I don't want to include the entire API. In particular, most of the types in antlr4ts/atn and antlr4ts/dfa are only useful in advanced scenarios, and including them in the root import would be very noisy. Here are the remaining ones:

• antlr4ts/misc: rarely used, probably just noise
• antlr4ts/tree: commonly used, and likely the primary motivation for creating this issue in the first place
• antlr4ts/tree/pattern: used in client applications, but much less common than the primary tree interfaces
• antlr4ts/tree/xpath: similar to antlr4ts/tree/pattern

❓ Would it be sufficient to simply export antr4ts/tree/index.ts from antlr4ts/index.ts?

[BurtHarris]

@sharwell, Yes, I think exporting antr4ts/tree/index.ts by default would be a big positive step. I'll give it a try then dust off the cobwebs and reply further.