Skip to content

Conversation

Fatme
Copy link
Contributor

@Fatme Fatme commented Oct 1, 2018

PR Checklist

What is the current behavior?

No error message is shown when installeld nativescript-dev-webpack plugin does not support hmr option or preview-sync hook.

What is the new behavior?

An error message is shown when installeld nativescript-dev-webpack plugin does not support hmr option or preview-sync hook.

Copy link
Contributor

@rosen-vladimirov rosen-vladimirov left a comment

Choose a reason for hiding this comment

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

Please add tests with some prerelease versions of nativescript-dev-webpack, for example:
0.17.0-2018-09-28-173604-01, 0.18.0-2018-09-28-173604-01, etc.

}

if (minSupportedVersion) {
const currentVersion = hasBundlerPluginAsDependency || hasBundlerPluginAsDevDependency;
Copy link
Contributor

Choose a reason for hiding this comment

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

the names of the properties seems incorrect, maybe bundlerVersionInDependencies and bundlerVersionInDevDependencies seems more appropriate

const currentVersion = hasBundlerPluginAsDependency || hasBundlerPluginAsDevDependency;
const isBundleSupported = semver.gte(semver.coerce(currentVersion), semver.coerce(minSupportedVersion));
if (!isBundleSupported) {
this.$errors.fail(util.format(BundleValidatorMessages.NotSupportedVersion, minSupportedVersion));
Copy link
Contributor

Choose a reason for hiding this comment

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

failWithoutHelp maybe, the command help may not help here

@Fatme Fatme force-pushed the famte/validate-bundle-version branch 2 times, most recently from 0ad4f6b to f6e6532 Compare October 4, 2018 12:00
expectedError: null
},
{
name: `should not throw an error when webpack's version is grather than minSupportedVersion when webpack is installed as ${key}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

is greater

@Fatme Fatme force-pushed the famte/validate-bundle-version branch from f6e6532 to 1502957 Compare October 5, 2018 05:14
@Fatme Fatme merged commit c8694d8 into master Oct 5, 2018
@Fatme Fatme deleted the famte/validate-bundle-version branch October 5, 2018 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants