Skip to content

Upgrade typescript version#954

Merged
johnnyreilly merged 5 commits intoTypeStrong:masterfrom
fa93hws:master
Jun 17, 2019
Merged

Upgrade typescript version#954
johnnyreilly merged 5 commits intoTypeStrong:masterfrom
fa93hws:master

Conversation

@fa93hws
Copy link
Copy Markdown
Contributor

@fa93hws fa93hws commented Jun 13, 2019

Latest version typescript has more strict type check.

Comment thread src/servicesHost.ts Outdated
@andrewbranch
Copy link
Copy Markdown
Contributor

Yeah, I don't think the loop is really worth it. Personally I’d probably go with something like

function addCache(servicesHost: typescript.ModuleResolutionHost): {
  moduleResolutionHost: typescript.ModuleResolutionHost,
  clearCache: () => void,
} {
  const clearCacheFunctions: Action[] = [];
  return {
    moduleResolutionHost: {
      ...servicesHost,
      fileExists: createCache(servicesHost.fileExists),
      directoryExists: servicesHost.directoryExists && createCache(servicesHost.directoryExists),
      realpath: servicesHost.realpath && createCache(servicesHost.realpath),
    },
    clearCache: () => clearCacheFunctions.forEach(clear => clear()),
  };

  function createCache<TOut>(originalFunction: (arg: string) => TOut) {
    const cache = new Map<string, TOut>();
    clearCacheFunctions.push(() => cache.clear());
    return function getCached(arg: string) {
      let res = cache.get(arg);
      if (res !== undefined) {
        return res;
      }

      res = originalFunction(arg);
      cache.set(arg, res);
      return res;
    };
  }
}

and then at the usage site

  let clearCache: null | (() => void) = null;
  let moduleResolutionHost: ModuleResolutionHost = {
    fileExists,
    readFile: readFileWithFallback,
    realpath: compiler.sys.realpath,
    directoryExists: compiler.sys.directoryExists,
    getCurrentDirectory: compiler.sys.getCurrentDirectory,
    getDirectories: compiler.sys.getDirectories
  };

  if (enableFileCaching) {
    const cached = addCache(moduleResolutionHost);
    clearCache = cached.clearCache;
    moduleResolutionHost = cached.moduleResolutionHost;
  }

I guess that’s slightly more wordy than what’s proposed here, but doesn’t contain any unsafe type assertions.

@johnnyreilly
Copy link
Copy Markdown
Member

Thanks for the suggestion @andrewbranch!

@fa93hws do you fancy tweaking your PR with the suggested implementation?

@andrewbranch
Copy link
Copy Markdown
Contributor

I feel like I should add that there are of course lots of different solutions that largely come down to personal preference of coding style. Avoiding unnecessary type assertions is a worthwhile goal, but I’m sure mine is not the most elegant possible solution.

@fa93hws
Copy link
Copy Markdown
Contributor Author

fa93hws commented Jun 17, 2019

No problem!

Copy link
Copy Markdown
Member

@johnnyreilly johnnyreilly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Do you want to update the CHANGELOG.md? Remember to thank yourself 😁

Comment thread package.json
"tslint-config-prettier": "^1.15.0",
"typescript": "^3.1.1",
"typescript": "^3.5.1",
"webpack": "^4.5.0",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we go for TypeScript 3.5.2 since it's out?

Copy link
Copy Markdown
Contributor Author

@fa93hws fa93hws Jun 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem!
The version goes so quick

@johnnyreilly johnnyreilly merged commit 757137a into TypeStrong:master Jun 17, 2019
@johnnyreilly
Copy link
Copy Markdown
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants