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

[WIP] Migrate build to Vite #2959

Closed
wants to merge 8 commits into from
Closed

[WIP] Migrate build to Vite #2959

wants to merge 8 commits into from

Conversation

vinicius73
Copy link
Contributor

@vinicius73 vinicius73 commented Aug 5, 2022

A proof of concept to use Vite instead Webpack.

Vite is a build tool that aims to provide a faster and leaner development experience for modern web projects.

https://vitejs.dev/guide/why.html

  • Add Vite build process
  • Remove webpack from build
    • Migrate from styleguidist to another tool

This change works as expected without any specific errors (regards problems from v6-alfa version whom this PR is based)

nextcloud/text#2779


ESM Build

ECMAScript Modules and Tree Shaking1 are largely used nowadays.

We do not need to target our packages as CommonJS modules only.

Webpack, Vite, Rollup, Parcel and Skypack are able to detect what kind of module we need to use.

Also, we have a good compatibility layer.

ESM Native support

The main browsers are already compatible with ECMAScript Modules2. Also, Node.js is natively compatible.

  • Chrome >=87
  • Firefox >=78
  • Safari >=13
  • Edge >=88

Bundlers like Vite target ESM as default for browsers and node environments.

This change improves the performance and bundle's size of our packages, since a lot of transpiration can be removed from the final bundle.

Break Changes

No changes in any component have made. But, the import of components and functions had changes.

- import Actions from '@nextcloud/vue/dist/Components/Actions'
- import ActionButton from '@nextcloud/vue/dist/Components/ActionButton'
+ import { Actions, ActionButton } from '@nextcloud/vue' 

All components and functions are exposed directly in the main file. We do not need any more import each component or function file.

It not mean what an unused component will be bundled in the final application.
The Tree Shaking1 process take care of this and remove any dead code.

Emoji functions

As all functions are directly expose, the function addRecent from src/functions/emoji have chanced to make clear what kind of function it is.

Now addRecent is called emojiAddRecent

CSS

Vite generate CSS file separated from JavaScript files. We need to import it in or applications manually.

import '@nextcloud/vue/style.css'

Footnotes

  1. webpack, Roolup and parcel 2

  2. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules

@vinicius73 vinicius73 added 2. developing Work in progress dependencies Pull requests that update a dependency file breaking PR that requires a new major version labels Aug 5, 2022
vite.config.js Outdated Show resolved Hide resolved
Comment on lines +6 to +7
// eslint-disable-next-line no-lone-blocks
{
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious why you added this lone block. Does it have any effect? I don't see any scoped variables in there. They are all in the TRANSLATIONS.forEach loop anyway.

Is it related to the last example with the source type module here: https://eslint.org/docs/latest/rules/no-lone-blocks ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to avoid some inconsistencies when replace this entry with an array.

My idea is remove part of this code and avoid some unnecessary computations.

Copy link
Contributor

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

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

Just a brief code review for now. Have not tried building / using this myself.

I also wonder about the timeline... will we ship this with a new major release that we then start using for NC 26?

@vinicius73
Copy link
Contributor Author

I also wonder about the timeline... will we ship this with a new major release that we then start using for NC 26?

I don't know, the main block is styleguidist.
The build its self is working perfectly, but to make send migrate to Vite, keep webpack just for styleguidist do not look reasonable to me.

I will try https://histoire.dev/, but it is a large refactoring.

@vinicius73 vinicius73 force-pushed the feature/vite branch 2 times, most recently from 23f82f6 to e477db9 Compare August 10, 2022 20:42
@vinicius73
Copy link
Contributor Author

image

I've been playing a bit with https://histoire.dev/
Works well, but some features are missing.

@skjnldsv
Copy link
Contributor

Works well, but some features are missing.

Seems like very important features, so let's not move away from it yet I'd say

@vinicius73 vinicius73 force-pushed the feature/vite branch 2 times, most recently from 2cfb435 to 5ce47f6 Compare August 22, 2022 13:51
@skjnldsv skjnldsv removed their request for review August 22, 2022 17:23
@vinicius73 vinicius73 force-pushed the feature/vite branch 3 times, most recently from efd55c1 to 72c8374 Compare August 29, 2022 15:38
@vinicius73 vinicius73 force-pushed the feature/vite branch 4 times, most recently from cd7c6e4 to 263dfdc Compare September 1, 2022 12:31
Vinicius Reis added 8 commits September 12, 2022 09:50
Signed-off-by: Vinicius Reis <vinicius.reis@nextcloud.com>
Signed-off-by: Vinicius Reis <vinicius.reis@nextcloud.com>
Signed-off-by: Vinicius Reis <vinicius.reis@nextcloud.com>
Signed-off-by: Vinicius Reis <vinicius.reis@nextcloud.com>
Signed-off-by: Vinicius Reis <vinicius.reis@nextcloud.com>
Signed-off-by: Vinicius Reis <vinicius.reis@nextcloud.com>
Signed-off-by: Vinicius Reis <vinicius.reis@nextcloud.com>
Signed-off-by: Vinicius Reis <vinicius.reis@nextcloud.com>
@skjnldsv
Copy link
Contributor

skjnldsv commented Nov 3, 2022

Vite generate CSS file separated from JavaScript files. We need to import it in or applications manually.

Does that means we now have one big css instead of tiny ones?
If so, maybe consider this https://www.npmjs.com/package/vite-plugin-css-injected-by-js

@susnux
Copy link
Contributor

susnux commented Nov 29, 2022

@vinicius73 about styleguideist, long story short: I overlooked your work here and also created a vite branch including API documentation using vue-doc-gen and vitepress.

Besides switching from webpack to vite I also switched from jest to vitest and from styleguidist to doc-gen + vitepress.
The only feature missing is the live preview of the components, because it is really a pain to have vue2 and vue3 installed in parallel (vitepress using vue3 and @nextcloud/vue components working with vue2).
Locally I could get it work, but only using experimental node functions (import maps).

Maybe you can get some inspiration from my branch: https://github.com/nextcloud/nextcloud-vue/compare/enh/vite?expand=1

@skjnldsv
Copy link
Contributor

@susnux can you open a PR from it?
So we can have a look?

@susnux
Copy link
Contributor

susnux commented Nov 30, 2022

can you open a PR from it? So we can have a look?

@skjnldsv of cause, #3529

@raimund-schluessler
Copy link
Contributor

Closing in favor of #3565

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress breaking PR that requires a new major version dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants