Importing interface from a file with an enum (or a class)

[dballesteros7]

Consider the following files:

types.ts

export interface ITokenStorageService {
  getToken():string;
  setToken(token:string):void;
}

export const enum LoginState {
  LogginIn, LoggedIn, NotLoggedIn
}

token.ts

import {ITokenStorageService} from './types';

export default class TokenStorageService implements ITokenStorageService {

  private userToken:string;

   getToken():string {
    return this.userToken;
  }

  setToken(token:string) {
    this.userToken = token;
  }
}

When compiling this on the browser, the following error appears:

importerModule.setters[importerIndex] is not a function

I traced it to https://github.com/systemjs/systemjs/blob/master/lib/register.js#L287.

The problem is that the transpiled token.js file has no setters but the types file is registered as a dependency of the token module. I think this has to do with JSPM and how it registers the imports even if typescript doesn't register the dependency at runtime?

For now I just added a bandaid fix:

if (importerIndex < importerModule.setters.length) {
  importerModule.setters[importerIndex](exports);
}

Any ideas? Thanks!

[frankwallis]

@dballesteros7 thanks for raising this, it is a rather tricky issue, but I have captured it in a test now, so just need to work out what the best fix is. It happens because the TypeScript compiler elides this import as it is not needed at runtime; but plugin-typescript injects it manually as it is needed for type-checking. This causes the issue you are seeing in systemjs.

@guybedford the issue here is that when instantiating a system.register module, SystemJS is including the extra dependencies received in metadata.deps and if they have exports, then it is trying to call a setter which does not exist (it goes off the end of the setters array).

I can think of a couple of ways to fix this:

1. Have SystemJS check the length of the setters array (as in comment above)
2. plugin-typescript can workaround this by overriding the instantiate hook and only injecting the extra dependencies after instantiation. This is doable but seems a bit hacky.

Do you foresee any issues with implementing (1), if not I can send a PR?

[guybedford]

@frankwallis the idea with metadata.deps or injecting import statements is that it adds dependencies into the dependency tree. Having exceptions that treat some metadata.deps as not part of that tree breaks the definition of what those deps are, so I don't think we can remove things there.

This seems to have come from our previous discussion at #97 (comment). There the metadata.deps approach was in response to code that needed to be injected at runtime. Typings themselves should probably not be running in this way?

Again perhaps it's a problem of my trying to make suggestions without understanding the full scope of the problems. Please do clarify further if you can!

[frankwallis]

@dballesteros7 I have applied a fix for the issue, so I think this should be working in 4.0.15

@guybedford these dependencies are dependencies for the translate stage of the pipeline but they are not runtime dependencies and at runtime it would be better if they were not emitted at all.

What plugin-typescript is trying to do is inject files into the pipeline which are needed in the translate stage and then at instantiate time remove files which are not needed. I want these dependencies to be recorded in the module registry so that things like hot reloading can see these dependencies as well.

Some other approaches I have tried:

1. Manually fetch these files outside of the regular pipeline (adds complexity, possibility of double fetches?, dependencies not available to hot-reloader)
2. Inject them with System.import/System.load (causes issues with files being executed at build-time)

[guybedford]

@frankwallis System.import in the builder itself is the best way to load non-runtime dependencies in the translate hook as it allows loading outside of the module pipeline, and it also ensures that loading runs at build time. Injecting dependencies into the tree is specifically for code that is part of the runtime execution itself (although conditional loading does allow distinguishing which dependencies in runtime are features been dev and production).

Being able to hot reload these build-level System.import calls is actually more something to consider as a hot reloading API problem as opposed to a System / builder API problem. I guess the problem is not when the code file changes, but when the typing file changes here? In such a case, if the hot reloader could provide an event API to notify when a file reloads, as well as an invalidation API, you would be able to listen to that event in the plugin itself, and do a manual invalidation of the file that was linked to it.

[dballesteros7]

@frankwallis It works for me with 4.0.16. Thanks! 👍