-
Notifications
You must be signed in to change notification settings - Fork 3
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
[Theming] Fix tsc output #498
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
"dist", | ||
"lib", | ||
"lib/constants", | ||
"lib/icons", | ||
"lib/utils" | ||
], | ||
"types": "./lib/index.d.ts", | ||
|
@@ -30,7 +31,7 @@ | |
"build:icons:deprecated": "svgr --expand-props none --template src/utils/svgToIconTemplate/index.js -d src/icons/ src/svgs/deprecated/", | ||
"build:icons:index": "create-index src/icons/", | ||
"build:rollup": "rollup -c", | ||
"build:babel": "babel src -d lib --extensions '.js,.ts,.tsx'", | ||
"build:babel": "babel src -d lib --extensions '.js,.ts,.tsx' --ignore '**/*.test.{js,ts,tsx}'", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We currently build test files, which seems unnecessary/inadvisable. I got side-tracked a bit about keeping actual source vs. test separate, could be a good Cooldown item to get things up to Open Source Best Practices. |
||
"build:svgs:index": "create-index src/svgs/ --recursive --update --extensions svg", | ||
"publish-package": "run-s build && yarn publish", | ||
"heroku-postbuild": "yarn run build:icons && yarn run build:icons:index && yarn run build-storybook", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
We write test helpers in `src/tests` instead of the `tests` directory at the root because in order for our TypeScript declaration files to not be deeply nested (e.g. `lib/src/constants`, instead of `lib/constants`, which is where we output our non-declaration source), `src` must be the "The longest common path of all non-declaration input files". | ||
|
||
See: https://www.typescriptlang.org/tsconfig#outDir and https://www.typescriptlang.org/tsconfig#rootDir |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
We write global `types` in `src/types` instead of a `types` directory at the root because in order for our TypeScript declaration files to not be deeply nested (e.g. `lib/src/constants`, instead of `lib/constants`, which is where we output our non-declaration source), `src` must be the "The longest common path of all non-declaration input files". | ||
|
||
See: https://www.typescriptlang.org/tsconfig#outDir and https://www.typescriptlang.org/tsconfig#rootDir |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{ | ||
"extends": "../tsconfig", | ||
"compilerOptions": {}, | ||
"references": [ | ||
{ | ||
"path": "../src" | ||
} | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,19 @@ | ||
import { configure } from 'enzyme'; | ||
import Adapter from 'enzyme-adapter-react-16'; | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call | ||
configure({ adapter: new Adapter() }); | ||
|
||
window.matchMedia = | ||
window.matchMedia || | ||
function () { | ||
return { | ||
matches: false, | ||
dispatchEvent: () => false, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fleshed out these properties to remove the TypeScript error, and I guess related changes made the |
||
addListener: () => undefined, | ||
removeListener: () => undefined, | ||
media: '', | ||
onchange: () => undefined, | ||
addEventListener: () => undefined, | ||
removeEventListener: () => undefined, | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,16 +3,13 @@ | |
"compilerOptions": { | ||
"baseUrl": "./", | ||
"declaration": true, | ||
"declarationMap": true, | ||
"emitDeclarationOnly": true, | ||
"outDir": "lib", | ||
"rootDirs": ["src"] | ||
// The folder structure in outDir is based on "The longest common path of all non-declaration input files." | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comments are valid |
||
// To match our babel output, we need to compile everything in `src` without also nesting it in a folder named `src`. | ||
// Setting the `rootDir` to `src` will guarantee this behavior, and raise compilation errors if violated. | ||
"rootDir": "src" | ||
}, | ||
"include": [ | ||
"src/**/*.ts", | ||
"src/**/*.js", | ||
"src/**/*.tsx", | ||
"tests/**/*.ts", | ||
"tests/**/*.tsx", | ||
"types/**/*.ts" | ||
] | ||
"include": ["src/**/*.ts", "src/**/*.js", "src/**/*.tsx"] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We import directly from here, so I think it should also be present here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, it seems you have to manually specify the nested paths here so I don't know how it was working. I remember I had to specify in daylight exactly
"files": [ "dist", "dist/static", "dist/static/media" ],
otherwise it would not output the media folder . But whatever this seems consistent