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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: extendPackage object values should be string #5038

Merged
merged 1 commit into from Jan 13, 2020
Merged

fix: extendPackage object values should be string #5038

merged 1 commit into from Jan 13, 2020

Conversation

@pksunkara
Copy link
Collaborator

pksunkara commented Jan 6, 2020

Fixes the following issue:


馃殌  Invoking generator for vue-cli-plugin-storybook...
 ERROR  TypeError: Cannot read property 'match' of undefined
TypeError: Cannot read property 'match' of undefined
    at resolveDeps (/home/circleci/vue-cli-plugin-storybook/node_modules/@vue/cli/lib/util/mergeDeps.js:14:27)
    at GeneratorAPI.extendPackage (/home/circleci/vue-cli-plugin-storybook/node_modules/@vue/cli/lib/GeneratorAPI.js:195:20)
    at module.exports (/home/circleci/vue-cli-plugin-storybook/tmp/node_modules/vue-cli-plugin-storybook/generator/index.js:9:7)
    at Generator.initPlugins (/home/circleci/vue-cli-plugin-storybook/node_modules/@vue/cli/lib/Generator.js:150:13)
    at Generator.generate (/home/circleci/vue-cli-plugin-storybook/node_modules/@vue/cli/lib/Generator.js:168:16)
    at runGenerator (/home/circleci/vue-cli-plugin-storybook/node_modules/@vue/cli/lib/invoke.js:124:19)
    at process._tickCallback (internal/process/next_tick.js:68:7)


What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Underlying tools
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

Other information:

@pksunkara pksunkara requested a review from sodatea Jan 6, 2020
@sodatea

This comment has been minimized.

Copy link
Member

sodatea commented Jan 6, 2020

In which case it would be a non-string and why should we accept non-string value?

@pksunkara

This comment has been minimized.

Copy link
Collaborator Author

pksunkara commented Jan 6, 2020

We should kick it up to the package manager. Right now, this is crashing unexpectedly.

@sodatea

This comment has been minimized.

Copy link
Member

sodatea commented Jan 6, 2020

Makes sense. Please add a test here:

test('api: warn invalid dep range', async () => {

@sodatea sodatea added the PR: Bug Fix label Jan 9, 2020
@pksunkara pksunkara force-pushed the extendPackage branch from 52e6400 to 8d8a83a Jan 13, 2020
@pksunkara

This comment has been minimized.

Copy link
Collaborator Author

pksunkara commented Jan 13, 2020

Updated. Before the fix, the test will fail as shown below:

 FAIL  packages/@vue/cli/__tests__/Generator.spec.js
  鈼 api: warn invalid dep range when non-string

    TypeError: Cannot read property 'match' of null

      13 |     const r2IsString = typeof r2 === 'string'
      14 |     const sourceGeneratorId = sources[name]
    > 15 |     const isValidURI = r2.match(/^(?:file|git|git\+ssh|git\+http|git\+https|git\+file|https?):/) != null
         |                           ^
      16 |     const isValidGitHub = r2IsString && r2.match(/^[^/]+\/[^/]+/) != null
      17 | 
      18 |     // if they are the same, do nothing. Helps when non semver type deps are used

      at mergeDeps (packages/@vue/cli/lib/util/mergeDeps.js:15:27)
      at GeneratorAPI.extendPackage (packages/@vue/cli/lib/GeneratorAPI.js:195:20)
      at apply (packages/@vue/cli/__tests__/Generator.spec.js:288:13)
      at Generator.initPlugins (packages/@vue/cli/lib/Generator.js:150:13)
      at Generator.generate (packages/@vue/cli/lib/Generator.js:168:16)
      at Object.<anonymous> (packages/@vue/cli/__tests__/Generator.spec.js:297:19)

Now, the test will pass.

@sodatea sodatea merged commit 1c269d2 into dev Jan 13, 2020
8 checks passed
8 checks passed
ci/circleci: cli-ui Your tests passed on CircleCI!
Details
ci/circleci: group-1 Your tests passed on CircleCI!
Details
ci/circleci: group-2 Your tests passed on CircleCI!
Details
ci/circleci: group-3 Your tests passed on CircleCI!
Details
ci/circleci: group-4 Your tests passed on CircleCI!
Details
ci/circleci: install Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@sodatea sodatea deleted the extendPackage branch Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can鈥檛 perform that action at this time.