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

chore(vite): Migrate from webpack to vite #2321

Merged
merged 1 commit into from
Dec 10, 2023
Merged

Conversation

raimund-schluessler
Copy link
Member

@raimund-schluessler raimund-schluessler commented Aug 21, 2023

This migrates the build from webpack to vite. This saves a bit on bundle size, but we have to include every stylesheet manually.

@raimund-schluessler

This comment was marked as resolved.

@raimund-schluessler

This comment was marked as resolved.

@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Merging #2321 (f812a6a) into master (c2d6f98) will increase coverage by 0.04%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2321      +/-   ##
==========================================
+ Coverage   24.79%   24.83%   +0.04%     
==========================================
  Files          63       63              
  Lines        2396     2392       -4     
  Branches      258      257       -1     
==========================================
  Hits          594      594              
+ Misses       1785     1780       -5     
- Partials       17       18       +1     

@susnux
Copy link

susnux commented Oct 3, 2023

Edit: Reading the provided link helps 🙈 The config should be a mjs instead of a js file. Might be worth to adjust this in @nextcloud/vite-config.

Not sure what you mean, I do not think there is anything to adjust in the config package.
I recommend every app to simply move to ESM (set type: module in package.json), so that .js is ESM by default (Most apps already do this but with transpiling their "ESM" like sources).

ESM projects can always import CJS, but CJS can not require ESM packages (only dynamically import them).

@susnux
Copy link

susnux commented Oct 3, 2023

@raimund-schluessler

The build now works, but the app does not load. No error in console. Do you already have an app working with this config, and could provide me a link to it?

Edit: Loading mjs files is blocked, renaming to js does not fix it.

What do you mean by blocked? Module JS (aka mjs aka ESM) is only supported with Nextcloud 27 and newer (there was a bug in 27.0.0 so 27.0.1 or newer).

If you have 27.0.1+ it should work as expected, as then Nextcloud will add type=module to the script tag for .mjs files to enable module support. This is automatically configured also in the .htaccess for apache (to send the correct mime type), for NGinx administrators should follow the upgrade guide in the docs to adjust the Nginx config.

As an example you could look as the logreader app which is already using that feature.

@raimund-schluessler
Copy link
Member Author

What do you mean by blocked? Module JS (aka mjs aka ESM) is only supported with Nextcloud 27 and newer (there was a bug in 27.0.0 so 27.0.1 or newer).

My dev environment is server latest master, but it could be that the webserver is not configured correctly, I guess. I get this error message when tasks-main.mjs is requested:

Loading module from “https://localhost/apps2/tasks/js/tasks-main.mjs” was blocked because of a disallowed MIME type (“application/octet-stream”). tasks

I didn't try with my production environment yet, though.

@susnux
Copy link

susnux commented Oct 4, 2023

My dev environment is server latest master, but it could be that the webserver is not configured correctly, I guess. I get this error message when tasks-main.mjs is requested:

Exactly this error means the webserver does not send the correct mime type for mjs files.
Apache gets configured automatically, as the .htaccess file was updated in Nextcloud 27, for Nginx you have to do this manually.

https://docs.nextcloud.com/server/stable/admin_manual/release_notes/upgrade_to_27.html?highlight=upgrade#web-server-configuration

@raimund-schluessler

This comment was marked as resolved.

@raimund-schluessler

This comment was marked as resolved.

@raimund-schluessler
Copy link
Member Author

And the dynamic import of TaskBody.vue does not work yet.

With @nextcloud/vite-config@1.1.0 this works now.

Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
@raimund-schluessler raimund-schluessler merged commit 798026d into master Dec 10, 2023
18 checks passed
@delete-merged-branch delete-merged-branch bot deleted the feat/noid/vite branch December 10, 2023 19:25
@raimund-schluessler raimund-schluessler modified the milestones: 0.17.0, 0.16.0 Dec 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants