aduth / tannin

gettext localization library compatible with Jed-formatted locale data
MIT License
13 stars 5 forks source link

Build: Use a composite TypeScript setup #4

Closed sirreal closed 4 years ago

sirreal commented 4 years ago

Reworks TypeScript setup as composite project.

See TypeScript documentation on project references: https://www.typescriptlang.org/docs/handbook/project-references.html

Project references are a new feature in TypeScript 3.0 that allow you to structure your TypeScript programs into smaller pieces.

By doing this, you can greatly improve build times, enforce logical separation between components, and organize your code in new and better ways.

This project has nice logical separation into several packages, the composite TypeScript setup helps to reflect that. Additionally, it allows for different packages do have differing compiler options. I've relaxed strictness only as much as necessary to get the project building without changes.

sirreal commented 4 years ago

For my purposes of prioritization, can you clarify if this is intended primarily as a refactoring? Or does it somehow block you with WordPress/gutenberg#18942 ?

This is entirely a refactoring for your consideration. Simply exploring this in a smaller, simpler context was helpful for me and my understanding of composite projects. This is low priority and not blocking any of my work. Please feel free to continue with it if you find it valuable or close if you're not interested 🙂

I'll reply to the rest of your feedback either way.

aduth commented 4 years ago

I'm having some trouble with this, specifically when running npm run build.

> @tannin/root@ build:types /Users/andrew/Documents/Code/tannin
> tsc --build packages/*

packages/compile/index.js:1:21 - error TS7016: Could not find a declaration file for module '@tannin/postfix'. '/Users/andrew/Documents/Code/tannin/packages/postfix/build/index.js' implicitly has an 'any' type.
  Try `npm install @types/tannin__postfix` if it exists or add a new declaration (.d.ts) file containing `declare module '@tannin/postfix';`

1 import postfix from '@tannin/postfix';
                      ~~~~~~~~~~~~~~~~~

packages/compile/index.js:2:22 - error TS7016: Could not find a declaration file for module '@tannin/evaluate'. '/Users/andrew/Documents/Code/tannin/packages/evaluate/build/index.js' implicitly has an 'any' type.
  Try `npm install @types/tannin__evaluate` if it exists or add a new declaration (.d.ts) file containing `declare module '@tannin/evaluate';`

2 import evaluate from '@tannin/evaluate';
                       ~~~~~~~~~~~~~~~~~~

I haven't been able to dig into it much yet. I'd guess it's (wrongly?) picking up on the files in build/ which aren't there for a clean install, which is why it works okay in a clean clone.

sirreal commented 4 years ago

I'm having some trouble with this, specifically when running npm run build.

It appears I got a few of the project references wrong as you notices. I've done another pass and they should be fixed. With this problem, tsc was unable to correctly determing the order in which projects should be built, resulting in missing declarations.

The references in tsconfig.json should correspond exactly to file:… dependencies in package.json.

sirreal commented 4 years ago

In addition to bad project references, the glob from de6b2cac64e7dfdb340bb8153b80e8ef5e3f02da was bad. This resulted in tsbuildinfo files existing and preventing generation of declaration files, failing the build. I've fixed this.

I also pushed a change that follows the project structure of https://github.com/RyanCavanaugh/project-references-demo, documented in the ce88d5a33b9014134d95f17aa75bb780823b65f1 commit message.