Update dependencies and restructure webpack config #130

ang-zeyu · 2020-02-20T11:11:02Z

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [x] New feature

What is the rationale for this request?
As mentioned here #118 (comment), the version of vue-loader vue-strap is using is outdated since support for it was dropped long ago, which prevents us from using some useful features:

deep selectors for scoped css
functional components which can help with performance ( and hence FOUC )

To upgrade vue-loader, almost all devDependencies had to be updated to accomodate webpack 4, which, going forward, can help with some things:

We can use miniCssExtractPlugin to load vue-strap styles in the <head> of html files, which is a standard technique in production to reduce FOUC.
Additionally, once we merge vue-strap into markbind as a scoped package, we can:
- tree shake bootstrap vue to only import the components we actually need (popovers, modals, tooltips)
- Introduce hot-reload ( or at least live reload ) with webpack-dev-server when we change source files to test changes, which can be used with markbind serve -o to accelerate the development process
- integrate a bundling + minification process when running markbind build for asset files, helping with FOUC
- ...

What changes did you make? (Give an overview)

Dependencies upgraded:

vue-loader
webpack ( vue-loader needs it )
webpack loader packages ( babel, css, less, sass, expose, ... ), consequently less and sass packages
eslint-plugin-vue ( for new vue-loader features ), and consequently all other eslint plugins for compatibility

Dependencies removed:

prismjs ( we arent using this )
uglifyJs ( TerserPlugin is the recommended default minifier which is included with webpack now, which has better performance and slightly better results ( -10kb for the minified bundle ))
vue-html-loader ( vue-loader no longer needs it )

Dependencies added:

webpack-merge ( highly used package to provide a semantic way to seperate dev and build webpack configs )

Webpack configs and npm scripts:

rewrote webpack configs to comply with many webpack v1 - v4 changes
split webpack configs with a commonly used structure ( common, dev, production ) using webpack-merge
Small QoL improvement: npm run build now only uses the production webpack-merge config which builds the minified css file.
- npm run builddev builds the unminified files with sourcemaps ( we aren't using these currently, but I preserved it as it may be useful once we merge vue-strap into markbind for certain tasks )

Testing instructions:

Delete your node_modules folder and npm/yarn install
npm run build and copy over the minified bundle, components should work as per usual. ( components page in docs would be a good place to test )

Proposed commit message: (wrap lines at 72 characters)
Update dependencies and restructure webpack config

The vue-strap fork is using highly outdated dependencies, which results
in being unable to use some new features of vue-loader which can be
helpful in development.

Let’s update these dependencies, and restructure the webpack configs to
accommodate webpack 4.
Let’s also refactor the npm build script to only build the minified
bundle, while also preserving the ability to build the non-minified
bundle with a builddev script.

openorclose · 2020-02-21T03:12:42Z

About treeshaking bootstrap-vue:

Let's raise this as an issue first.

Currently it's an undocumented? feature that authors can use any bootstrap-vue components in their site.

openorclose · 2020-02-21T03:58:46Z

Do we need both yarn.lock and package-lock.json?

ang-zeyu · 2020-02-21T05:45:02Z

About treeshaking bootstrap-vue:

Let's raise this as an issue first.

Currently it's an undocumented? feature that authors can use any bootstrap-vue components in their site.

Good idea, will post all the other points as issues as well once this MarkBind/markbind#981 is done.

Do we need both yarn.lock and package-lock.json?

https://classic.yarnpkg.com/blog/2018/06/04/yarn-import-package-lock/
Don't think we want devs to have to run yarn import individually

yamgent · 2020-02-22T04:32:27Z

Apologies if I missed something... Is there a reason why we still need the non-minfied version? Afaik the non-minfied version has never been used anywhere, we just inherited it from the original project (which is no longer been updated).

ang-zeyu · 2020-02-22T06:58:28Z

Apologies if I missed something... Is there a reason why we still need the non-minfied version? Afaik the non-minfied version has never been used anywhere, we just inherited it from the original project (which is no longer been updated).

It can be useful in the future once this is a scoped package in /markbind, I'm preserving it as more of a skeleton right now.

For e.g., I'm thinking of ( will be raised as issues once its a scoped package ):

setting up the dev config to use the unminified version with sourcemaps which allows us to use vue devtools properly and can further help with build times ( minification takes time ).
reconfiguring the dev command so that changes to @markbind/vue-strap files will automatically build and be copied to the template site(s) in /markbind, so no manual copying of files / writing our own copy scripts are required.

marvinchin

Mostly LGTM, just a question :)

Do we use yarn or npm as the package manager for markbind or vue-strap? I think it would make sense for us to stick to just one, so we won't need to keep updating both lockfiles. It would make our dev environment more consistent as well.

ang-zeyu · 2020-04-02T16:01:08Z

Mostly LGTM, just a question :)

Do we use yarn or npm as the package manager for markbind or vue-strap? I think it would make sense for us to stick to just one, so we won't need to keep updating both lockfiles. It would make our dev environment more consistent as well.

I think most devs here use npm since the main repo uses it as well; Should we remove yarn here?

marvinchin · 2020-04-02T16:32:39Z

Should we remove yarn here?

I think we should be fine to remove it. But let's check with @yamgent to see if there's any historical context for it's existence.

yamgent · 2020-04-04T05:22:59Z

But let's check with @yamgent to see if there's any historical context for it's existence.

I am unaware of anybody using yarn other than the original MarkBind developer (the original project that we fork from doesn't use yarn either). It is fine to remove yarn.lock, the npm one is more important.

The vue-strap fork is using highly outdated dependencies, which results in being unable to use some new features of vue-loader which can be helpful in development. Let’s update these dependencies, and restructure the webpack configs to accommodate webpack 4. Let’s also refactor the npm build script to only build the minified bundle, while also preserving the ability to build the non-minified bundle with a builddev script.

ang-zeyu · 2020-04-04T09:33:28Z

It is fine to remove yarn.lock, the npm one is more important.

Updated with yarn removed

marvinchin

LGTM 🚀

ang-zeyu · 2020-04-04T13:51:57Z

Thanks for reviewing this! @openorclose @yamgent @marvinchin

ang-zeyu force-pushed the webpack-vueloader-upgrade branch 2 times, most recently from 546a729 to 14d86bb Compare February 25, 2020 09:13

ang-zeyu mentioned this pull request Feb 26, 2020

Render navbar at compile time using a template MarkBind/markbind#1054

Closed

ang-zeyu force-pushed the webpack-vueloader-upgrade branch from 14d86bb to 54eb2ba Compare March 19, 2020 09:07

ang-zeyu requested a review from yamgent March 19, 2020 09:08

ang-zeyu force-pushed the webpack-vueloader-upgrade branch from 54eb2ba to bff651e Compare April 1, 2020 15:17

ang-zeyu requested a review from marvinchin April 1, 2020 15:27

marvinchin reviewed Apr 2, 2020

View reviewed changes

ang-zeyu force-pushed the webpack-vueloader-upgrade branch from bff651e to c8805e7 Compare April 4, 2020 09:30

ang-zeyu requested a review from marvinchin April 4, 2020 09:34

marvinchin approved these changes Apr 4, 2020

View reviewed changes

marvinchin merged commit 4811dc6 into MarkBind:master Apr 4, 2020

marvinchin added this to the v2.0.1-markbind.40 milestone Apr 4, 2020

marvinchin mentioned this pull request Apr 16, 2020

Update the build steps in the maintainer guide MarkBind/markbind#1204

Merged

This was referenced Apr 20, 2020

Upgrade MarkBind and vue-strap library dependencies MarkBind/markbind#461

Closed

VueStrap: Investigate using Storybook MarkBind/markbind#1212

Closed

Migrate all MarkBind components from vue-strap to bootstrap-vue MarkBind/markbind#814

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dependencies and restructure webpack config #130

Update dependencies and restructure webpack config #130

ang-zeyu commented Feb 20, 2020 •

edited

Loading

openorclose commented Feb 21, 2020

openorclose commented Feb 21, 2020

ang-zeyu commented Feb 21, 2020

yamgent commented Feb 22, 2020

ang-zeyu commented Feb 22, 2020

marvinchin left a comment

ang-zeyu commented Apr 2, 2020

marvinchin commented Apr 2, 2020

yamgent commented Apr 4, 2020

ang-zeyu commented Apr 4, 2020

marvinchin left a comment

ang-zeyu commented Apr 4, 2020

Update dependencies and restructure webpack config #130

Update dependencies and restructure webpack config #130

Conversation

ang-zeyu commented Feb 20, 2020 • edited Loading

Dependencies upgraded:

Dependencies removed:

Dependencies added:

Webpack configs and npm scripts:

openorclose commented Feb 21, 2020

openorclose commented Feb 21, 2020

ang-zeyu commented Feb 21, 2020

yamgent commented Feb 22, 2020

ang-zeyu commented Feb 22, 2020

marvinchin left a comment

Choose a reason for hiding this comment

ang-zeyu commented Apr 2, 2020

marvinchin commented Apr 2, 2020

yamgent commented Apr 4, 2020

ang-zeyu commented Apr 4, 2020

marvinchin left a comment

Choose a reason for hiding this comment

ang-zeyu commented Apr 4, 2020

ang-zeyu commented Feb 20, 2020 •

edited

Loading