feat(common): Add dev-mode sanity check for mismatched versions of cdk and material #15395

macjohnny · 2019-03-06T08:58:03Z

Add dev-mode sanity check for mismatched versions of cdk and material and show console warning if versions don't match.

…k and material Add dev-mode sanity check for mismatched versions of cdk and material and show console warning if versions don't match. Fixes #15387

macjohnny · 2019-03-06T13:47:02Z

@jelbourn the bazel builds fail due to the @angular/cdk index file not being exported in some test configurations and in
src/lib/core/common-behaviors/common-module.ts I import the cdk version as follows:

import {VERSION as CDK_VERSION} from '@angular/cdk';

it would be much easier to export the cdk version in a secondary entry point, too, e.g.

import {VERSION as CDK_VERSION} from '@angular/cdk/bidi';

would you accept that? the same applies to the material version...

or could you give me a hand fixing the remaining bazel configurations that are failing?

src/lib/core/common-behaviors/common-module.ts

…s of cdk and material fix version comparison

jelbourn · 2019-03-06T21:35:26Z

src/lib/core/common-behaviors/common-module.ts

@@ -104,6 +107,18 @@ export class MatCommonModule {
    this._document.body.removeChild(testElement);
  }

+  /** Checks whether the material version matches the cdk version */
+  private _checkCdkVersionMatch(): void {
+    if (VERSION.major !== CDK_VERSION.major && VERSION.minor !== CDK_VERSION.minor) {


I actually want to go so far as to say that the versions have to exactly match. You could do this by comparing VERSION.full !== CDK_VERSION.full

@jelbourn I updated the PR to compare the full version. could you please tell me how you want me to proceed concerning the test-build issues (see my comment above: #15395 (comment))?

I exported an additional symbol export const ɵVERSION in the @angular/cdk/bidi module and added another non-exportet VERSION symbol in the common-module.ts, otherwise it would be necessary to adapt various build configs to allow accessing the VERSION symbol from @angular/material and @angular/cdk.
The build and tests now work without failures.

This shouldn't be necessary. @devversion could you advise on the build issue here?

@macjohnny Yeah. You should be able to import the version from @angular/material and @angular/cdk. In order to allow this, we need to adjust the tsconfig path mapping (like you already did with your initial commit).

Also for Bazel, you need to update the ng_module definition here. Just add //src/cdk there (before //src/cdk/a11y).

Though it will be a bit more difficult for the version from Material. We don't want core to depend on the @angular/material primary entry-point which transitively depends on all Material secondary entry-points (e.g. slide-toggle, list etc.)

Maybe the best would be to just use the version placeholder in core (like it's done right now) with a proper comment. Eventually we can remove this once the Material primary entry-point no longer re-exports all secondary entry-points (see how the CDK entry-point works)

@devversion @jelbourn I updated the PR according to your suggestions. the build now works without additionally exporting the VERSION symbol. could you have a look at it again?

…versions of cdk and material fix version comparison

…atched versions of cdk and material export version in secondary entry point to avoid testing issues

…or mismatched versions of cdk and material fix testing / build issues

…s of cdk and material fix build issues

…s of cdk and material fix upgrade issues

…s of cdk and material refactor version export

…ode-cdk-material-version-check

…s of cdk and material fix linting, fix prerender build

…s of cdk and material code cleanup

jelbourn · 2019-03-11T16:44:20Z

src/lib/core/common-behaviors/common-module.ts

+          'The Angular Material version ' + VERSION.full + ' does not match ' +
+          'the Angular CDK version ' + (CDK_VERSION ? CDK_VERSION.full : '') + '.\n' +
+          'Please install Angular CDK ' + VERSION.full + ': ' +
+          'npm install @angular/cdk@' + VERSION.full


I would tweak the message to be:

The Angular Material version (xxx) does not match the Angular CDK version (yyy). Please ensure the versions of these two packages exactly match.

Since people might want to keep their cdk version and change material. Also people might be using yarn, so I would avoid giving an actual command.

I updated the message accordingly

…s of cdk and material change warning message

devversion

The tsconfig mapping should be only necessary for the src/lib/tsconfig-build.json, but it doesn't hurt.

LGTM from my side.

src/cdk/tsconfig-tests.json

macjohnny · 2019-03-11T17:31:13Z

@devversion the various tsconf mappings are necessary for

the IDE during development
running the tests
running the prerender
Removing any will cause at least one test/build to fail

devversion · 2019-03-11T17:37:21Z

@macjohnny It's okay having these configs. Though I initially wasn't sure why it would be necessary in the universal-app since nothing explicitly imports from the @angular/cdk primary entry-point.

For later reference: This is because of some SSR workaround, which requires us to copy the release output into the dist/ and build everything from there (therefore the typings need the path mapping)

jelbourn

LGTM, thanks!

…ode-cdk-material-version-check

macjohnny · 2019-05-22T05:32:06Z

@jelbourn will this make it into the v8 release?

jelbourn · 2019-05-22T16:10:13Z

Probably will be 8.1 at this point

…n-check

src/material/core/common-behaviors/common-module.ts

…k and material (angular#15395)

angular-automatic-lock-bot · 2019-09-10T23:01:23Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

macjohnny requested review from devversion and jelbourn as code owners March 6, 2019 08:58

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Mar 6, 2019

feat(common): Add dev-mode sanity check for mismatched versions of cd…

9711223

…k and material Add dev-mode sanity check for mismatched versions of cdk and material and show console warning if versions don't match. Fixes #15387

crisbeto reviewed Mar 6, 2019

View reviewed changes

src/lib/core/common-behaviors/common-module.ts Outdated Show resolved Hide resolved

fixup! feat(common): Add dev-mode sanity check for mismatched version…

92aa360

…s of cdk and material fix version comparison

jelbourn reviewed Mar 6, 2019

View reviewed changes

macjohnny added 10 commits March 7, 2019 07:21

fixup! fixup! feat(common): Add dev-mode sanity check for mismatched …

82bee97

…versions of cdk and material fix version comparison

fixup! fixup! fixup! feat(common): Add dev-mode sanity check for mism…

6686411

…atched versions of cdk and material export version in secondary entry point to avoid testing issues

fixup! fixup! fixup! fixup! feat(common): Add dev-mode sanity check f…

a4a9b29

…or mismatched versions of cdk and material fix testing / build issues

fixup! feat(common): Add dev-mode sanity check for mismatched version…

c4212bc

…s of cdk and material fix build issues

fixup! feat(common): Add dev-mode sanity check for mismatched version…

dd76739

…s of cdk and material fix build issues

fixup! feat(common): Add dev-mode sanity check for mismatched version…

a0478a1

…s of cdk and material fix upgrade issues

fixup! feat(common): Add dev-mode sanity check for mismatched version…

377d01d

…s of cdk and material refactor version export

Merge remote-tracking branch 'origin/master' into feature/15387-dev-m…

5b8ddf9

…ode-cdk-material-version-check

fixup! feat(common): Add dev-mode sanity check for mismatched version…

87c8ecf

…s of cdk and material fix linting, fix prerender build

fixup! feat(common): Add dev-mode sanity check for mismatched version…

6287258

…s of cdk and material code cleanup

jelbourn reviewed Mar 11, 2019

View reviewed changes

fixup! feat(common): Add dev-mode sanity check for mismatched version…

cb1a21b

…s of cdk and material change warning message

devversion approved these changes Mar 11, 2019

View reviewed changes

src/cdk/tsconfig-tests.json Show resolved Hide resolved

devversion added the pr: lgtm label Mar 11, 2019

jelbourn approved these changes Mar 27, 2019

View reviewed changes

jelbourn added action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels Mar 27, 2019

jelbourn added ༼ つ ◕_◕ ༽つ and removed ༼ つ ◕_◕ ༽つ labels Apr 25, 2019

mmalerba added the aaa label Apr 25, 2019

mmalerba removed the aaa label Apr 25, 2019

Merge remote-tracking branch 'origin/master' into feature/15387-dev-m…

3324173

…ode-cdk-material-version-check

andrewseguin added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label May 30, 2019

Merge branch 'master' into feature/15387-dev-mode-cdk-material-versio…

d8fe557

…n-check

josephperrott reviewed Jun 5, 2019

View reviewed changes

src/material/core/common-behaviors/common-module.ts Show resolved Hide resolved

josephperrott added action: merge The PR is ready for merge by the caretaker and removed action: merge The PR is ready for merge by the caretaker labels Jun 5, 2019

josephperrott merged commit ffad004 into angular:master Jun 6, 2019

RudolfFrederiksen pushed a commit to RudolfFrederiksen/material2 that referenced this pull request Jun 21, 2019

feat(common): Add dev-mode sanity check for mismatched versions of cd…

dee2f8d

…k and material (angular#15395)

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019

feat(common): Add dev-mode sanity check for mismatched versions of cdk and material #15395

feat(common): Add dev-mode sanity check for mismatched versions of cdk and material #15395

Uh oh!

Conversation

macjohnny commented Mar 6, 2019

Uh oh!

macjohnny commented Mar 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devversion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

macjohnny commented Mar 11, 2019

Uh oh!

devversion commented Mar 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

macjohnny commented May 22, 2019

Uh oh!

jelbourn commented May 22, 2019

Uh oh!

Uh oh!

angular-automatic-lock-bot bot commented Sep 10, 2019

Uh oh!

Uh oh!

macjohnny commented Mar 6, 2019 •

edited

Loading

devversion commented Mar 11, 2019 •

edited

Loading