Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(all): use typescript project references #632

Merged
merged 13 commits into from Feb 25, 2021
Merged

Conversation

unstubbable
Copy link
Member

@unstubbable unstubbable commented Feb 6, 2021

This also enables skipLibCheck which reduces the compile time significantly. It is also a recommended compiler option.

Note: Explicit usage of third-party code is still checked.

  • switch some scripts in packages/demos to js to avoid using ts-node (no support for project references)
  • fix TypeDoc integration
  • investigate change in size-limit result
    The increase of @feature-hub/core by 2 kb is caused by a newer version of semver (required by ts-loader) which introduced memoization to improve the performance of range parsing. With this lru-cache was added as a dependency which unfortunately leads to the larger overall size of @feature-hub/core.
  • investigate why Netlify can not build the API docs

@unstubbable unstubbable marked this pull request as draft February 6, 2021 17:17
@unstubbable unstubbable changed the base branch from master to fix-demo-dev-server February 7, 2021 17:04
@fahrradflucht
Copy link
Member

fahrradflucht commented Feb 13, 2021

I don't understand why they recommend this option - isn't it just unsafe? You don't know anymore for example if your dep references browser APIs that aren't in your matrix or anything like that.

@unstubbable
Copy link
Member Author

The code you are using is type checked. So if a browser API in these code paths is used that you don't support the compiler should be able to catch that (need to test that in a small repro). Of course this assumes that the lib itself would be free of side effects.

@fahrradflucht
Copy link
Member

💡 I didn't know that. So what you are saying is this?

// node_modules/a/index.ts
function foo(): string {
  return new Intl.PluralRules().select(0);
}

function bar(): string {
  return "other";

// src/works.ts
import { bar } from "a";

console.log(bar());

// src/broken.ts
import { foo } from "a";

console.log(foo());

Base automatically changed from fix-demo-dev-server to master February 16, 2021 11:17
@unstubbable
Copy link
Member Author

unstubbable commented Feb 16, 2021

@fahrradflucht It's a good thing you started this discussion. Based on your example I created this test repo: https://github.com/unstubbable/skip-lib-check-test Quoting the README from there:

The documentation says:

TypeScript will type check the code you specifically refer to in your app’s source code.

What it does not mention, but should not be surprising now that I think about it, is that any library code that can not be compiled, but your app's source code refers to, will be regarded as any. Even with noImplicitAny set to true this does not yield a compile error.

This brings me to the conclusion that this compiler option should only be used a stop-gap measure (if at all) for a short period of time, when the types of two libraries (in this case webpack and @types/webpack-dev-middleware) are incompatible with each other.

This reduces the compile time significantly. It is also a recommended
compiler option.

Note: Explicit usage of third-party code is still checked.
Since ts-node does not support project references we use js files with jsdoc type annotations instead.
This is needed to properly handle project references.
The increase is caused by a newer version of semver which introduced
memoization to improve the performance of range parsing. With this
lru-cache was added as a dependency which unfortunately leads to the
larger overall size of @feature-hub/core.
@unstubbable
Copy link
Member Author

unstubbable commented Feb 16, 2021

Removed skipLibCheck again in 5e6161b. There is currently no issue without skipLibCheck, until we decide to update to webpack 5, and DefinitelyTyped/DefinitelyTyped#50773 is not resolved till then.

There is currently no issue without `skipLibCheck`, until we decide to
update to webpack 5, and
DefinitelyTyped/DefinitelyTyped#50773 is not
resolved till then.
@unstubbable
Copy link
Member Author

unstubbable commented Feb 16, 2021

Interesting, with skipLibCheck removed, TypeDoc fails to generate the API docs with FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory somewhere in node_modules/typescript/lib/typescript.js. 🙄 EDIT: Seems to be a common issue: TypeStrong/typedoc#1346

@unstubbable unstubbable marked this pull request as ready for review February 17, 2021 06:40
packages/demos/src/node-integrator.js Outdated Show resolved Hide resolved
tsconfig.settings.json Show resolved Hide resolved
clebert
clebert previously approved these changes Feb 25, 2021
@unstubbable unstubbable merged commit 38ab27d into master Feb 25, 2021
@unstubbable unstubbable deleted the project-references branch February 25, 2021 12:47
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.

None yet

3 participants