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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: replace treeShake with globalImports #239

Merged
merged 8 commits into from
Jan 17, 2020

Conversation

husayt
Copy link

@husayt husayt commented Jan 5, 2020

treeShake has been renamed to globalImports everywhere except for plugin.js This is fixed with this pull request

treeShake has been renamed to globalImports everywhere except for plugin.js This is fixes with this pull request
@codecov
Copy link

codecov bot commented Jan 5, 2020

Codecov Report

Merging #239 into next will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@         Coverage Diff         @@
##           next   #239   +/-   ##
===================================
  Coverage   100%   100%           
===================================
  Files         7      7           
  Lines        64     64           
  Branches     19     19           
===================================
  Hits         64     64

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9f14e8...224de15. Read the comment docs.

@kevinmarrec
Copy link
Member

kevinmarrec commented Jan 9, 2020

nice catch @husayt , but seems there's something to do as well around plugin registration : https://github.com/nuxt-community/vuetify-module/blob/next/src/plugin.ts#L23 , globalImports should be given instead of treeShake

@husayt
Copy link
Author

husayt commented Jan 16, 2020

@kevinmarrec this is done now

The Vuetify module will always now import Vuetify from vuetify/lib which will provide natural tree shaking with Webpack.
src/plugin.ts Show resolved Hide resolved
templates/plugin.js Outdated Show resolved Hide resolved
@husayt
Copy link
Author

husayt commented Jan 16, 2020

@kevinmarrec seems you were looking at old changes. Can you check again pls

@kevinmarrec
Copy link
Member

@husayt Still doesn't look good to me in "Files" tab 🤷, is it GH issue or ?

@husayt
Copy link
Author

husayt commented Jan 17, 2020

Can you point to what is wrong @kevinmarrec . I actually did those changes directly in github as I didn't have a chance to edit files locally. Feel free to change what doesn't look right.

@kevinmarrec
Copy link
Member

Well the things you marked as resolved are not resolved :

  • Vuetify instead of Vue
  • Missing globalImports when given options to plugin

@husayt
Copy link
Author

husayt commented Jan 17, 2020

Indeed, you are right. Something is weird with Github editor, I am sure I did those changes yesterday. Nevermind, all should be fine now.

Copy link
Member

@kevinmarrec kevinmarrec left a comment

Choose a reason for hiding this comment

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

Thanks @husayt 😉

@kevinmarrec kevinmarrec added the fix Pull requests that fixes something label Jan 17, 2020
@kevinmarrec kevinmarrec changed the title fix inconsistency by replacing treeShake with globalImports fix: replace treeShake with globalImports Jan 17, 2020
@kevinmarrec kevinmarrec merged commit 48e6bef into nuxt-community:next Jan 17, 2020
@kevinmarrec
Copy link
Member

kevinmarrec commented Jan 17, 2020

I will release later today :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Pull requests that fixes something
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants