This repository was archived by the owner on Mar 4, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 53
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
59c8812
to
7799633
Compare
aneeshack4
reviewed
Jan 16, 2020
a7c8a36
to
e593373
Compare
e593373
to
a393613
Compare
Changed dependencies are detected.Changed dependencies in
Perf comparison
Generated by 🚫 dangerJS |
@layershifter @kenotron This appears to be done (build succeeded)! Please take a look when you get a chance. |
layershifter
approved these changes
Jan 29, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Standardize patterns around custom types (with the goal of making it easier to add or modify types or debug issues):
Remove the
typeRoots
config fromtsconfig.common.json
and only settypeRoots
in individual packages that need custom types. And remove places thatinclude
-ed the maintypes
directory, since I think having it bothinclude
-ed and used as atypeRoot
may confuse the compiler and/or editors.Make the
types
directory follow the standard structure for a type root (<package name>/index.d.ts
).Add dependencies on
@types/webpack-env
and remove things from the custom global types which properly come from there.Move the global types file to
docs
since that's the only place it's used.Wrap
screener-runner
types in a module, and only import it from the projects that need it. For now it still exposes types globally, but eventually all the files referencing those types should be updated to use explicit imports.Remove most custom types:
@types/classnames
@types/simulant
which I wrote 😄 (and move mainsimulant
dep to appropriate places)anchor-js
types since they weren't doing anything, andts-ignore
that importRemove
react
fromtsconfig
types
lists since it shouldn't be included ambiently.Move
docs
,e2e
, andperf
tsconfig
s to their respective projects instead of having them inbuild
(same thing will be done with webpack configs later).Add types to webpack configs.
Notes on using package names in
tsconfig
extends
pathsIn theory we should be able to do
"extends": "@fluentui/internal-tooling/tsconfig.common.json"
instead of"../../build/tsconfig.common.json"
. However, this can cause multiple problems...typeRoots
orbaseUrl
is specified in the shared config, it will get resolved relative tonode_modules/@fluentui/internal-tooling/tsconfig.common.json
(because TS doesn't use realpaths when resolving configs). This is bad because it makes a lot of types and modules get resolved incorrectly.typeRoots
andbaseUrl
from the shared config, I ran into some other issues (I think with module resolution)...unfortunately I don't remember the details, and it may be solvable, but I decided to stick with relative paths for now because there are too many other things to deal with.I think the lack of realpath resolution is a bug or at least a desirable enhancement, so I'm going to file an issue with TS about that. (Naturally I'm having trouble getting an isolated repro...)