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

📦 Switch to Yarn workspaces; overhaul build system #1741

Merged
merged 26 commits into from
Nov 16, 2017
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
1b33ffb
Switch to yarn workspaces, upgrade some dev dependencies
adidahiya Oct 19, 2017
738f313
[core] fix typescript compilation
adidahiya Oct 19, 2017
a73de92
Remove require-shim.d.ts files, we now use @types/node
adidahiya Oct 19, 2017
cc924a2
[table] fix typescript compilation
adidahiya Oct 19, 2017
9cf4847
update tsconfig.json files
adidahiya Oct 19, 2017
ea85b65
[docs] fix typescript compilation
adidahiya Oct 19, 2017
0d7765e
[labs] fix typescript compilation
adidahiya Oct 19, 2017
9e88abe
[core] fix some tests
adidahiya Oct 19, 2017
567f51f
bump to webpack 3!
Oct 24, 2017
2e6b4a4
fix ts-loader errors
Oct 24, 2017
80efeb3
clean yarn.lock
Oct 24, 2017
57a6ee0
massage package.json versions to hoist as much as possible
Oct 24, 2017
0670962
upgrade site-landing and table preview to webpack 3
Oct 24, 2017
2eaa4f4
get karma webpacking
Oct 24, 2017
ddc54b0
karma serves/includes correct files
Oct 25, 2017
a127b30
:star: test compile errors now fail the build!
Oct 25, 2017
acac406
fix core and datetime tests
Oct 25, 2017
dba77e7
fix webpack-watch
Oct 25, 2017
9efda93
quick ignore ContextMenu component cuz it's untestable (not exported)
Oct 25, 2017
057f1de
bump range slider test coverage
Oct 25, 2017
98bcc9c
Fix Table test typings
cmslewis Oct 26, 2017
285f318
Comment out broken tests :/
cmslewis Oct 26, 2017
33f5eba
Merge remote-tracking branch 'origin/master' into ad/workspaces
adidahiya Oct 31, 2017
ab3b459
Merge remote-tracking branch 'origin/master' into ad/workspaces
adidahiya Nov 14, 2017
83ffd5e
Refactor build system, remove Gulp (#1786)
adidahiya Nov 16, 2017
1a3d563
Remove Gulpfile.js
adidahiya Nov 16, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .yarnrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
workspace-experimental true
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ The other packages (`site-docs` and `site-landing`) are not published to NPM as
We use [Lerna](https://lernajs.io/) to manage inter-package dependencies in this monorepo.
Builds are orchestrated via [Gulp](http://gulpjs.com/) tasks.

__Prerequisites__: Node.js v6+, Yarn v0.28+
__Prerequisites__: Node.js v8+, Yarn v1.0+

1. `git clone` this repository (or fork if you lack permissions).
1. `yarn` to install dependencies at the root of the repo.
Expand Down
15 changes: 4 additions & 11 deletions circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,29 +21,22 @@ machine:
pre:
- mkdir ~/yarn
environment:
YARN_VERSION: 0.28.4
YARN_VERSION: 1.2.1
PATH: $PATH:$HOME/$CIRCLE_PROJECT_REPONAME/node_modules/.bin
node:
version: 7.10.0
version: 8.5.0

dependencies:
cache_directories:
- ~/yarn
- ~/.cache/yarn
- ~/$CIRCLE_PROJECT_REPONAME/packages/core/node_modules
- ~/$CIRCLE_PROJECT_REPONAME/packages/datetime/node_modules
- ~/$CIRCLE_PROJECT_REPONAME/packages/docs/node_modules
- ~/$CIRCLE_PROJECT_REPONAME/packages/labs/node_modules
- ~/$CIRCLE_PROJECT_REPONAME/packages/site-docs/node_modules
- ~/$CIRCLE_PROJECT_REPONAME/packages/site-landing/node_modules
- ~/$CIRCLE_PROJECT_REPONAME/packages/table/node_modules
# non-zero exit codes in `dependencies` group will fail the build early
# so these following commands will block the build and prevent tests
override:
- curl -o- -L https://yarnpkg.com/install.sh | bash -s -- --version $YARN_VERSION
- yarn install --pure-lockfile
- echo "Checking if yarn.lock changed..." && git diff --exit-code
- yarn
- yarn bootstrap
- echo "Checking if lockfiles changed..." && git diff --exit-code
- yarn build:gulp

test:
Expand Down
3 changes: 2 additions & 1 deletion lerna.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"lerna": "2.0.0",
"lerna": "2.4.0",
"npmClient": "yarn",
"useWorkspaces": true,
"version": "independent"
}
83 changes: 43 additions & 40 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
"version": "1.32.0",
"private": true,
"description": "A React UI toolkit for the web.",
"workspaces": [
"packages/*"
],
"scripts": {
"bootstrap": "lerna bootstrap",
"build:landing": "(cd packages/site-landing; npm run build)",
Expand All @@ -16,23 +19,23 @@
"serve": "http-server docs"
},
"dependencies": {
"@types/assertion-error": "1.0.30",
"@types/chai": "3.5.2",
"@types/classnames": "0.0.31",
"@types/dom4": "1.5.20",
"@types/enzyme": "2.8.0",
"@types/mocha": "2.2.32",
"@types/pure-render-decorator": "0.2.27",
"@types/react": "0.14.40",
"@types/react-addons-css-transition-group": "0.14.17",
"@types/react-addons-transition-group": "0.14.17",
"@types/react-dom": "15.5.0",
"@types/tether": "1.1.27",
"autoprefixer": "^7.1.2",
"@types/assertion-error": "^1.0.30",
"@types/chai": "^4.0.4",
"@types/classnames": "^2.2.3",
"@types/dom4": "^1.5.20",
"@types/enzyme": "^2.8.0",
"@types/mocha": "^2.2.43",
"@types/pure-render-decorator": "^0.2.28",
"@types/react": "^16.0.14",
"@types/react-addons-css-transition-group": "^15.0.3",
"@types/react-addons-transition-group": "^15.0.1",
"@types/react-dom": "^16.0.1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎆 awesome sauce! although might it be more accurate to use the latest 15.x types instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The types are basically the same and it's much easier to upgrade everything to latest; we won't be able to use new features at runtime anyway since we run react 15 in development.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely buy the upgrade/similarity arguments, but my concern with this change is that new features are available at compile time but not at run time.

if you're not concerned then I'm chill.

"@types/tether": "^1.4.3",
"autoprefixer": "^7.1.5",
"better-handlebars": "github:wmeldon/better-handlebars",
"chai": "^4.1.0",
"chai": "^4.1.2",
"del": "^3.0.0",
"documentalist": "^0.0.6",
"documentalist": "^0.0.8",
"enzyme": "^2.9.1",
"gulp": "^3.9.1",
"gulp-concat": "^2.6.1",
Expand All @@ -46,60 +49,60 @@
"gulp-rename": "^1.2.2",
"gulp-replace": "^0.6.1",
"gulp-sass": "^3.1.0",
"gulp-sourcemaps": "^2.6.0",
"gulp-sourcemaps": "^2.6.1",
"gulp-strip-css-comments": "^1.2.0",
"gulp-stylelint": "^4.0.0",
"gulp-stylelint": "^5.0.0",
"gulp-tslint": "^8.1.2",
"gulp-typescript": "^3.2.1",
"gulp-typescript": "^3.2.2",
"gulp-util": "^3.0.8",
"highlights": "^3.0.1",
"http-server": "^0.10.0",
"istanbul-instrumenter-loader": "^0.2.0",
"json-loader": "^0.5.4",
"karma": "^1.7.0",
"istanbul-instrumenter-loader": "^3.0.0",
"json-loader": "^0.5.7",
"karma": "^1.7.1",
"karma-chai": "^0.1.0",
"karma-chrome-launcher": "^2.2.0",
"karma-coverage": "^1.1.1",
"karma-firefox-launcher": "^1.0.1",
"karma-mocha": "^1.3.0",
"karma-mocha-reporter": "^2.2.3",
"karma-mocha-reporter": "^2.2.5",
"karma-phantomjs-launcher": "^1.0.4",
"karma-phantomjs-shim": "^1.4.0",
"karma-phantomjs-shim": "^1.5.0",
"karma-sinon": "^1.0.5",
"karma-sourcemap-loader": "^0.3.7",
"karma-webpack": "^2.0.4",
"karma-webpack": "^2.0.5",
"language-less": "github:atom/language-less",
"language-typescript": "github:giladgray/language-typescript#10.1.15",
"lerna": "^2.0.0",
"lerna": "^2.4.0",
"lodash": "^4.17.4",
"marked": "^0.3.6",
"merge-stream": "^1.0.1",
"mocha": "^3.4.2",
"mocha": "^4.0.1",
"node-sass-package-importer": "^3.0.4",
"npm-run-all": "^4.0.2",
"phantomjs-prebuilt": "^2.1.14",
"postcss-import": "^10.0.0",
"postcss-url": "^7.1.1",
"prettier": "^1.7.0",
"npm-run-all": "^4.1.1",
"phantomjs-prebuilt": "^2.1.15",
"postcss-import": "^11.0.0",
"postcss-url": "^7.1.2",
"prettier": "^1.7.4",
"react": "^15.5.1",
"react-dom": "^15.5.1",
"react-test-renderer": "^15.5.4",
"run-sequence": "^2.0.0",
"run-sequence": "^2.2.0",
"sinon": "^1.17.6",
"sorted-object": "^2.0.1",
"source-map-loader": "^0.2.1",
"source-map-loader": "^0.2.2",
"string-replace-loader": "^1.3.0",
"stylelint": "^8.0.0",
"stylelint": "^8.2.0",
"stylelint-config-palantir": "^2.1.0",
"stylelint-config-standard": "^17.0.0",
"stylelint-scss": "^1.5.1",
"ts-loader": "^1.3.3",
"ts-quick-docs": "^0.5.1",
"stylelint-scss": "^2.1.0",
"ts-loader": "^3.0.3",
"ts-quick-docs": "^0.5.3",
"tslint": "^5.7.0",
"tslint-config-prettier": "^1.5.0",
"tslint-plugin-prettier": "^1.1.0",
"tslint-config-prettier": "^1.6.0",
"tslint-plugin-prettier": "^1.3.0",
"tslint-react": "^3.2.0",
"typescript": "~2.2.1",
"typescript": "~2.4.2",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😱 hooray! is this a safe change for .d.ts compatibility?

Copy link
Contributor Author

@adidahiya adidahiya Oct 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be safe since we're not using new features, but there are no guarantees. I would just highlight the small chance of breaking syntax in the release notes and if something does come up, file it as a low priority issue to fix in the future. I don't want to wait until the next major version to upgrade typescript.

"vinyl-source-stream": "^1.1.0",
"webpack": "^1.13.2"
},
Expand Down
3 changes: 2 additions & 1 deletion packages/core/examples/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "2.0.0",
"version": "2.4.2",
"compilerOptions": {
"baseUrl": ".",
"declaration": true,
Expand All @@ -12,6 +12,7 @@
"noImplicitReturns": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"pretty": true,
"removeComments": false,
"sourceMap": false,
"stripInternal": true,
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/common/abstractComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ export abstract class AbstractComponent<P, S> extends React.Component<P, S> {
* @returns a "cancel" function that will clear timeout when invoked.
*/
public setTimeout(callback: () => void, timeout?: number) {
const handle = setTimeout(callback, timeout);
const handle = window.setTimeout(callback, timeout);
this.timeoutIds.push(handle);
return () => clearTimeout(handle);
return () => window.clearTimeout(handle);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious: why add the window reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with dependency hoisting, we get @types/node at the root. the global setTimeout resolves to node's setTimeout before it sees window.setTimeout in lib.d.ts. so we make this more explicit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be cool to add a code comment to that effect

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's pretty obvious when you make the mistake. it's a compile error. the typedef takes you to a different file. not going to add a comment

Copy link
Contributor

@giladgray giladgray Oct 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you think about tslint banning non-window references to set/clearTimeout/Interval? particularly clearTimeout, which does not produce a compiler error.

}

/**
Expand All @@ -53,7 +53,7 @@ export abstract class AbstractComponent<P, S> extends React.Component<P, S> {
public clearTimeouts = () => {
if (this.timeoutIds.length > 0) {
for (const timeoutId of this.timeoutIds) {
clearTimeout(timeoutId);
window.clearTimeout(timeoutId);
}
this.timeoutIds = [];
}
Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/components/hotkeys/hotkeysDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ class HotkeysDialog {
this.hotkeysQueue.push(hotkeys);

// reset timeout for debounce
clearTimeout(this.showTimeoutToken);
this.showTimeoutToken = setTimeout(this.show, DELAY_IN_MS);
window.clearTimeout(this.showTimeoutToken);
this.showTimeoutToken = window.setTimeout(this.show, DELAY_IN_MS);
}

public hideAfterDelay() {
clearTimeout(this.hideTimeoutToken);
this.hideTimeoutToken = setTimeout(this.hide, DELAY_IN_MS);
window.clearTimeout(this.hideTimeoutToken);
this.hideTimeoutToken = window.setTimeout(this.hide, DELAY_IN_MS);
}

public show = () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/components/slider/coreSlider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ export abstract class CoreSlider<P extends ICoreSliderProps> extends AbstractCom

private updateTickSize() {
if (this.trackElement != null) {
const tickSize = this.trackElement.clientWidth / (this.props.max - this.props.min);
const tickSize = this.trackElement.clientWidth / ((this.props.max as number) - (this.props.min as number));
this.setState({ tickSize });
}
}
Expand Down
1 change: 0 additions & 1 deletion packages/core/src/components/tabs/tab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ export class Tab extends React.Component<ITabProps, {}> {
className={classNames(Classes.TAB, this.props.className)}
id={this.props.id}
role="tab"
selected={this.props.isSelected ? true : null}
tabIndex={this.props.isDisabled ? null : 0}
>
{this.props.children}
Expand Down
1 change: 0 additions & 1 deletion packages/core/src/components/tabs2/tabTitle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ export class TabTitle extends React.Component<ITabTitleProps, {}> {
id={generateTabTitleId(parentId, id)}
onClick={disabled ? undefined : this.handleClick}
role="tab"
selected={selected ? true : undefined}
tabIndex={disabled ? undefined : 0}
>
{this.props.title}
Expand Down
7 changes: 0 additions & 7 deletions packages/core/test/callout/calloutTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,6 @@ describe("<Callout>", () => {
assert.isTrue(wrapper.hasClass(Classes.INTENT_DANGER));
});

it("spreads HTML props", () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't find it very useful, and it was using invalid HTML props

const onClick = sinon.spy();
const wrapper = shallow(<Callout label="label" onClick={onClick} />);
assert.strictEqual(wrapper.prop("label"), "label");
assert.strictEqual(wrapper.prop("onClick"), onClick);
});

it("renders optional title element", () => {
const title = "I am the title";
const wrapper = shallow(<Callout title={title} />);
Expand Down
7 changes: 1 addition & 6 deletions packages/core/test/tag/tagTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,7 @@ describe("<Tag>", () => {
});

it("passes other props onto .pt-tag element", () => {
const element = shallow(
<Tag alt="foo bar" title="baz qux">
Hello
</Tag>,
).find(".pt-tag");
assert.deepEqual(element.prop("alt"), "foo bar");
const element = shallow(<Tag title="baz qux">Hello</Tag>).find(".pt-tag");
assert.deepEqual(element.prop("title"), "baz qux");
});

Expand Down
3 changes: 2 additions & 1 deletion packages/core/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "2.0.0",
"version": "2.4.2",
"compilerOptions": {
"declaration": true,
"experimentalDecorators": true,
Expand All @@ -13,6 +13,7 @@
"noUnusedLocals": true,
"noUnusedParameters": true,
"outDir": "dist/",
"pretty": true,
"removeComments": false,
"sourceMap": false,
"stripInternal": true,
Expand Down
Loading