📦 Switch to Yarn workspaces; overhaul build system #1741

adidahiya · 2017-10-19T06:10:31Z

Fixes #1031.
Fixes #116.
Fixes #436.
Fixes #1527.
Fixes #780.

See #1786 for more details about the build system overhaul.

Changes proposed in this pull request:

Upgrade dev setup to Node 8.5 and Yarn 1.0
- I did not upgrade engines in package.json because this all probably still works in Node 6... (no it's not worth testing for it in CI, if we get bug report we can just bump engines to the range we care about)
Switch to using Yarn workspaces
- This makes installs faster, forces us to be more consistent about sharing the same dependencies across packages, and makes lockfile resolution easier.
Bump various dev dependencies within existing semver ranges
Bump documentalist
Bump chai
Bump stylelint-scss
Bump ts-loader
Bump webpack from v1 to v3!!

adidahiya · 2017-10-19T16:35:26Z

I think our version of webpack doesn't like resolving node_modules at the root... or we might have to explicitly add ../../node_modules to the list of bundler resolution paths.

giladgray

love where this is going! mostly got some questions about reasoning behind some of these changes

giladgray · 2017-10-19T18:03:29Z

package.json

    "tslint-react": "^3.2.0",
-    "typescript": "~2.2.1",
+    "typescript": "~2.4.2",


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

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.

giladgray · 2017-10-19T18:03:57Z

packages/core/src/common/abstractComponent.ts

        this.timeoutIds.push(handle);
-        return () => clearTimeout(handle);
+        return () => window.clearTimeout(handle);


curious: why add the window reference?

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

would be cool to add a code comment to that effect

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

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

giladgray · 2017-10-19T18:04:30Z

packages/core/test/callout/calloutTests.tsx

@@ -29,13 +29,6 @@ describe("<Callout>", () => {
        assert.isTrue(wrapper.hasClass(Classes.INTENT_DANGER));
    });

-    it("spreads HTML props", () => {


why remove this test?

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

giladgray · 2017-10-19T18:05:31Z

packages/docs/src/tags/heading.tsx

@@ -14,7 +14,7 @@ const Heading: React.SFC<IHeadingTag> = ({ level, route, value }) =>
    React.createElement(
        `h${level}`,
        { className: "docs-title" },
-        <a className="docs-anchor" key="anchor" name={route} />,


wait I think this attribute is critical to functionality! there's a selector that checks the name attr (although something is currently broken #1682)

name is not a valid HTML anchor attribute according to the latest lib.d.ts

data-name then please

giladgray · 2017-10-19T18:06:47Z

packages/site-docs/package.json

@@ -1,5 +1,5 @@
 {
-  "name": "blueprintjs.com/docs",
+  "name": "@blueprintjs/site-docs",


idea behind the previous name was that it's obvious that the package is not published. I'm sure you ran into the issue where / is an invalid char in yarn package names. how about blueprintjs.com-docs?

I find it weird that this monorepo doesn't follow the standard of using a single NPM scope. "private": true should make that publishing fact obvious enough.

giladgray · 2017-10-19T18:07:17Z

packages/site-landing/src/require-shim.d.ts

-
-// borrowed from https://github.com/TypeStrong/ts-loader
-
-declare var require: {


awesome that we can delete this! where does the type info come from now?

@types/node at the root

giladgray · 2017-10-19T18:08:08Z

package.json

+    "@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",


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

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.

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.

remove localResolve support -- not necessary with hoisting

giladgray · 2017-10-25T01:46:53Z

@adidahiya I think the build stuff is now in a good state here! Tasks are working and development is possible. Now, all that remains should be some test fixes to get a passing build!

@cmslewis would you mind contributing fixes for the table tests?

adidahiya · 2017-10-25T14:37:42Z

recent changes lgtm

blueprint-bot · 2017-10-26T23:44:41Z

Comment out broken tests :/

Preview: documentation | landing | table
Coverage: core | datetime

giladgray

is it safe for me to approve this?

giladgray

not safe, as docs site does not load (table and landing are good though)

# Conflicts: # packages/site-docs/src/require-shim.d.ts # packages/site-landing/src/require-shim.d.ts # packages/table/preview/require-shim.d.ts

blueprint-bot · 2017-10-31T22:34:03Z

Merge remote-tracking branch 'origin/master' into ad/workspaces

Preview: documentation | landing | table
Coverage: core | datetime

blueprint-bot · 2017-11-14T17:01:35Z

Merge remote-tracking branch 'origin/master' into ad/workspaces

Preview: documentation | landing | table
Coverage: core | datetime

blueprint-bot · 2017-11-16T15:24:01Z

Remove Gulpfile.js

Preview: documentation | landing | table
Coverage: core | datetime

adidahiya · 2017-11-16T16:09:50Z

ready to go

cmslewis

🔥

preview sites look fine now

giladgray

🚢 🚢 🚢 🚢 🚢 🚢 🚢 🚢 🚢

adidahiya and others added 8 commits October 19, 2017 01:18

Switch to yarn workspaces, upgrade some dev dependencies

1b33ffb

[core] fix typescript compilation

738f313

Remove require-shim.d.ts files, we now use @types/node

a73de92

[table] fix typescript compilation

cc924a2

update tsconfig.json files

9cf4847

[docs] fix typescript compilation

ea85b65

[labs] fix typescript compilation

0d7765e

[core] fix some tests

9e88abe

adidahiya requested a review from giladgray October 19, 2017 16:35

giladgray reviewed Oct 19, 2017

View reviewed changes

Gilad Gray added 10 commits October 24, 2017 14:23

bump to webpack 3!

567f51f

remove localResolve support -- not necessary with hoisting

fix ts-loader errors

2e6b4a4

clean yarn.lock

80efeb3

massage package.json versions to hoist as much as possible

57a6ee0

upgrade site-landing and table preview to webpack 3

0670962

get karma webpacking

2eaa4f4

karma serves/includes correct files

ddc54b0

⭐ test compile errors now fail the build!

a127b30

fix core and datetime tests

acac406

fix webpack-watch

dba77e7

Gilad Gray added 2 commits October 24, 2017 19:18

quick ignore ContextMenu component cuz it's untestable (not exported)

9efda93

bump range slider test coverage

057f1de

cmslewis added 2 commits October 26, 2017 16:07

Fix Table test typings

98bcc9c

Comment out broken tests :/

285f318

giladgray approved these changes Oct 27, 2017

View reviewed changes

giladgray added the Domain: tooling label Oct 27, 2017

giladgray changed the title ~~[WIP] Switch to Yarn workspaces; upgrade some dev dependencies~~ Switch to Yarn workspaces; upgrade some dev dependencies Oct 27, 2017

giladgray changed the title ~~Switch to Yarn workspaces; upgrade some dev dependencies~~ 📦 Switch to Yarn workspaces; upgrade some dev dependencies Oct 27, 2017

giladgray previously requested changes Oct 27, 2017

View reviewed changes

Merge remote-tracking branch 'origin/master' into ad/workspaces

33f5eba

# Conflicts: # packages/site-docs/src/require-shim.d.ts # packages/site-landing/src/require-shim.d.ts # packages/table/preview/require-shim.d.ts

Merge remote-tracking branch 'origin/master' into ad/workspaces

ab3b459

adidahiya and others added 2 commits November 16, 2017 15:13

Refactor build system, remove Gulp (#1786)

83ffd5e

Remove Gulpfile.js

1a3d563

adidahiya changed the title ~~📦 Switch to Yarn workspaces; upgrade some dev dependencies~~ 📦 Switch to Yarn workspaces; overhaul build system Nov 16, 2017

cmslewis approved these changes Nov 16, 2017

View reviewed changes

giladgray approved these changes Nov 16, 2017

View reviewed changes

giladgray merged commit 858ceb1 into master Nov 16, 2017

giladgray deleted the ad/workspaces branch November 16, 2017 21:08

giladgray mentioned this pull request Nov 18, 2017

Table API examples missing from GitHub #1818

Closed

mattjenny mentioned this pull request Dec 19, 2017

Blueprint is incompatible with "@types/node" #1942

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

📦 Switch to Yarn workspaces; overhaul build system #1741

📦 Switch to Yarn workspaces; overhaul build system #1741

adidahiya commented Oct 19, 2017 •

edited

Loading

adidahiya commented Oct 19, 2017 •

edited

Loading

giladgray left a comment

giladgray Oct 19, 2017

adidahiya Oct 20, 2017 •

edited

Loading

giladgray Oct 19, 2017

adidahiya Oct 20, 2017

giladgray Oct 24, 2017

adidahiya Oct 24, 2017

giladgray Oct 24, 2017 •

edited

Loading

giladgray Oct 19, 2017

adidahiya Oct 20, 2017

giladgray Oct 19, 2017

adidahiya Oct 20, 2017

giladgray Oct 24, 2017

giladgray Oct 19, 2017

adidahiya Oct 20, 2017

giladgray Oct 19, 2017

adidahiya Oct 20, 2017

giladgray Oct 19, 2017

adidahiya Oct 20, 2017

giladgray Oct 24, 2017

giladgray commented Oct 25, 2017

adidahiya commented Oct 25, 2017

blueprint-bot commented Oct 26, 2017

giladgray left a comment

giladgray left a comment

blueprint-bot commented Oct 31, 2017

blueprint-bot commented Nov 14, 2017

blueprint-bot commented Nov 16, 2017

adidahiya commented Nov 16, 2017

cmslewis left a comment

giladgray left a comment


		// borrowed from https://github.com/TypeStrong/ts-loader

		declare var require: {

📦 Switch to Yarn workspaces; overhaul build system #1741

📦 Switch to Yarn workspaces; overhaul build system #1741

Conversation

adidahiya commented Oct 19, 2017 • edited Loading

Changes proposed in this pull request:

adidahiya commented Oct 19, 2017 • edited Loading

giladgray left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adidahiya Oct 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giladgray Oct 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giladgray commented Oct 25, 2017

adidahiya commented Oct 25, 2017

blueprint-bot commented Oct 26, 2017

Comment out broken tests :/

giladgray left a comment

Choose a reason for hiding this comment

giladgray left a comment

Choose a reason for hiding this comment

blueprint-bot commented Oct 31, 2017

Merge remote-tracking branch 'origin/master' into ad/workspaces

blueprint-bot commented Nov 14, 2017

Merge remote-tracking branch 'origin/master' into ad/workspaces

blueprint-bot commented Nov 16, 2017

Remove Gulpfile.js

adidahiya commented Nov 16, 2017

cmslewis left a comment

Choose a reason for hiding this comment

giladgray left a comment

Choose a reason for hiding this comment

adidahiya commented Oct 19, 2017 •

edited

Loading

adidahiya commented Oct 19, 2017 •

edited

Loading

adidahiya Oct 20, 2017 •

edited

Loading

giladgray Oct 24, 2017 •

edited

Loading