-
-
Notifications
You must be signed in to change notification settings - Fork 113
fix: add missing null-loader dependency #152
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be in dependencies of packages/vue-cli-plugin-vuetify
@KaelWD I did a local build and didn't see it complain, so just to make sure I follow are you referring to the following block? vue-cli-plugins/packages/vue-cli-plugin-vuetify/package.json Lines 34 to 37 in addc5bc
|
Great job @JamesMcMahon tracking this down and fixing it. I'm not sure why @KaelWD wants It would be great to get this merged. |
This PR adds
yes that is the correct location, but I think it should be under |
Yeah I can make those changes. The way you phrased it makes sense @DRoet. Thanks for clarifying. |
7c4aff2
to
31d3724
Compare
Fix issue in vuetifyjs#101 On the fresh project adding the `vue add vuetify` plugin results in a missing dependency: ``` WEBPACK Failed to compile with 9 error(s) Error in ./node_modules/vuetify/lib/components/VImg/VImg.js Module not found: 'null-loader' in '/Users/jamie/temp/vuetify-create-test' ```
31d3724
to
41ea4de
Compare
PR has been updated. I removed the dependency from ad-hoc and added to the package.json. |
Confirmed and tested locally that this works. Note for myself in case I need to make further changes, https://cli.vuejs.org/dev-guide/plugin-dev.html#installing-plugin-locally. |
I am just checking in on this. I see two approvals, is there anything I need to do before this is merged? |
nope, it just needs to get merged/released by someone who has the time to do it |
@johnleider Do you have some time to merge this in and do a release? |
Need to change the loader to use
sassRule.use('null-loader').loader(require.resolve('null-loader'))
scssRule.use('null-loader').loader(require.resolve('null-loader'))
|
Want to create a separate PR for that? I want to scope this PR down to just
installing the dependency.
This has been broken for 6 or so months now and this PR has been sitting
approved for two months so I would like to see it merged so we can build
additional needed changes on top of it.
…On Sun, Apr 26, 2020, 05:48 Kristoffer K. ***@***.***> wrote:
Need to change the loader to use require.resolve('null-loader') otherwise
we're depending on hoisting which is a terrible idea
https://github.com/vuetifyjs/vue-cli-plugins/blob/908f65d35478da2cd9df3569c2ce4c5ca1910468/packages/vue-cli-plugin-vuetify/index.js#L48
Should be
sassRule.use('null-loader').loader(require.resolve('null-loader'))
https://github.com/vuetifyjs/vue-cli-plugins/blob/908f65d35478da2cd9df3569c2ce4c5ca1910468/packages/vue-cli-plugin-vuetify/index.js#L52
Should be
scssRule.use('null-loader').loader(require.resolve('null-loader'))
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#152 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAG7ZRCHEM7YKSC2W4LV2CDROQGQXANCNFSM4KIQUB4A>
.
|
@KaelWD can we get this merged? :) |
@johnleider friendly ping |
Fix issue in #101
On the fresh project adding the
vue add vuetify
plugin results in a missing dependency: