microsoft / microsoft/fluentui

bug: TypeScript target/lib option doesn't constraint ECMA features on type checking within packages

Open
#17,101 5 comments 1 reaction 1 assignee View on GitHub

@Hotell is already working on this.

Since May 17, 2021.

Area: Build System Area: Packaging Area: Typescript Fluent UI react (v8) Needs: Discussion Resolution: Soft Close
Dominant language
TypeScript
Stars
20.3k
Forks
2.9k
Avg merge
2d 9h
Merged PRs (30d)
46

Description

Library

@fluentui/react - all v8 packages

Current Behaviour

While implementing https://github.com/microsoft/fluentui/pull/16976 I ran into following TS related(compile/runtime) bug within our packages.

  • target/lib tsconfig option restriction has effect on code that is used within monorepo package

image

Expected behavior:

I should get TS errors based on target/lib setting.

Why is this happening?

The issues starts within react-conformance package

image

  • because we are using ES2015 features, those 2 functions will infer return type based on node globals and thus will end up
    with following output
    image
  • node types are explicitly importing ECMA2015+ libs from TS standard lib thus adding those to TS eval scope
    • image

Now because this, any API that is used from this package will "extend" (pollute) TS eval scope of particular package where it's being used (in our example react-menu)

👉 That's why we don't see any errors in our react-menu (event though we are using ECMA2015+ features)

Let's say we'd fix consoleUtil.ts (implementing Object.assign on our own for example), the main problem would not go away.

Why?

  • if we briefly skim through react-conformance implementation, we would discover that there is enzyme imported (within defaultTests.tsx
  • now enzyme types add cheerio types (/// <reference types="cheerio" />) and cherio types add node types (/// <reference types="node" />) so we end up with the same issue

Conclusion
Now even if we remove enzyme and other related culprits, nothing will stop us to introduce such a anomaly in future, because we are mixing 2 environments (with storybook in place 3) within 1 TS config, those leaks like this are inevitable.

An elegant solution for this problem was already introduced in (this RFC), - tsconfig per environment consolidated via TS Solution style config.

Also the solution in more detail: https://gist.github.com/Hotell/19e776aa1a47258340b8bd9b477ad303

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.