Resolve relative imports in Protobufs compiled to JavaScript

[dmytro-grankin]

Currently, Protobuf compiler generates JavaScript files with relative imports, e.g:

var spine_options_pb = require('../../spine/options_pb.js');

It means that all the files used as dependencies should be generated for every module and cannot be reused. The PR solves the issue and allows to replace relative imports by global, library-like imports.

So, the import above can be changed to:

var spine_options_pb = require('spine-web/spine/options_pb.js');

The adjustment of imports is done by Proto JS plugin by the ResolveImports task.

Imports of standard Protobuf types

The task also adjusts imports of standard Protobuf types like require('google-protobuf/google/protobuf/any_pb.js'). These imports are made non-relative by Protobuf compiler. But we want to use standard types, which are additionally processed by the Proto JS plugin. So, the imports are adjusted to refer spine-web module instead google-protobuf.

The set of imports that will be adjusted depends on the configuration of the Proto JS plugin. By default, the plugin will adjust only imports of files provided by Spine via NPM.

Custom configuration of imports resolving

You can also specify the custom modules and directories they provide. So, the plugin will replace relative imports by imports of module files.

An example:

protoJs {
      modules = [
           // The module provides `company/client` directory (not including subdirectories).
           // So, an import path like {@code ../company/client/file.js}
           // becomes {@code client/company/client/file.js}.
           'client' : ['company/client'],
     
           // The module provides `company/server` directory (including subdirectories).
           // So, an import path like {@code ../company/server/nested/file.js}
           // becomes {@code server/company/server/nested/file.js}.
           'server' : ['company/server/*'],
     
           // The module provides 'proto/company` directory.
           // So, an import pah like {@code ../company/file.js}
           // becomes {@code common-types/proto/company/file.js}.
           'common-types' : ['proto/company']
     ]
}

For a more comprehensive description of the feature configuration, see the wiki page.

Other changes

The PR also solves #306 by introducing a new abstract base for plugins — ProtoPlugin. This class provides FileSet including the types from the module known_types.desc and from the project runtime dependencies and ensures that these types are present in KnownTypes.

Under the hood, ProtoPlugin uses FileDescriptorSuperset. So, this and related classes were moved from model-compiler module to plugin-base.

[codecov]

Codecov Report

Merging #298 into master will increase coverage by 0.82%.
The diff coverage is 79.02%.

@@             Coverage Diff              @@
##             master     #298      +/-   ##
============================================
+ Coverage     72.68%   73.51%   +0.82%     
- Complexity     2139     2207      +68     
============================================
  Files           348      353       +5     
  Lines          9223     9425     +202     
  Branches        582      595      +13     
============================================
+ Hits           6704     6929     +225     
+ Misses         2367     2343      -24     
- Partials        152      153       +1

[dmytro-grankin]

@armiol PTAL.

[armiol]

@dmytro-grankin please see my comments. The general issue is that you define basic terms poorly, and then all the code referencing the basic terms has unclear naming and, probably, mixed responsibility.

[armiol]

Please clarify, whether it's a path to the imported file or an import statement at whole.

[armiol]

With this name it looks Path, not String.

[armiol]

Strings is a typo.

Also, such a method in a public API looks weird. It looks a part of something more meaningful. Let's discuss.

[armiol]

Again, bad naming and bad explanation. It's not a path.

Same below.

[armiol]

Somewhere there should be a notion of what a resolution is.

[armiol]

Is it an intention to skip or checking if should be skipped?

Also, how "skip for a module" plays with "import path"? I am confused.

[armiol]

Let's not link anything which has no explanation anywhere.

[armiol]

Please rephrase "library-like". Did you mean the imports of Spine library files? If so, just write a longer sentence, but make it more comprehensible.

[armiol]

The link should be not here, but in a place, which describes how to override the default behavior.

[dmytro-grankin]

@armiol PTAL.

[armiol]

@dmytro-grankin LGTM.