tables icon indicating copy to clipboard operation
tables copied to clipboard

Move to vite for bundling

Open juliusknorr opened this issue 1 year ago • 11 comments

  • [x] Based on https://github.com/nextcloud/tables/pull/1328
  • [x] Introduce vite for building
  • [x] Drop 26 suppport as there is no support for javascript modules that are required by vite https://github.com/nextcloud/tables/pull/1179

juliusknorr avatar Feb 08 '24 16:02 juliusknorr

@juliushaertl You say "based on https://github.com/nextcloud/tables/pull/647", but I don't see how?

enjeck avatar Feb 09 '24 07:02 enjeck

I based my branch locally on top of yours and then created a pull request towards that one. You can also sett that in the github ui:

Screenshot 2024-02-09 at 09 22 56

Once your PR/branch gets merged github automatically changes this, but that way I can make use of your changes already during development.

juliusknorr avatar Feb 09 '24 08:02 juliusknorr

Doesn't work for me after running npm ci and then npm run dev. Am I missing something?

enjeck avatar Feb 09 '24 10:02 enjeck

Sorry, it isn't ready yet, didn't wanted to ask for review but the ones got autoassigned by github 🙈 I'll ping you once I'm ready :)

juliusknorr avatar Feb 09 '24 11:02 juliusknorr

I also noticed that tables have dark favicons for me now:

Just tried with current main with the same result. So not related to this PR.

max-nextcloud avatar Feb 13 '24 10:02 max-nextcloud

Likely caused by https://github.com/nextcloud/tables/pull/712 then

juliusknorr avatar Feb 13 '24 14:02 juliusknorr

We need to decide when to merge this, currently the vite build requires https://github.com/nextcloud/server/pull/36057 which is 27 only.

juliusknorr avatar Feb 13 '24 14:02 juliusknorr

@susnux I'm having some trouble here with the npm run watch command which always exits after the first build. Is that something you've seen already? Any clue why that might happen? We just call vite --mode development build --watch

juliusknorr avatar Feb 14 '24 07:02 juliusknorr

@susnux I'm having some trouble here with the npm run watch command which always exits after the first build.

Should be fixed now: https://github.com/vitejs/vite/issues/15951

susnux avatar Mar 12 '24 16:03 susnux

I think the ?? '' can just be removed in this case - but it would also be nice to catch such warnings in the ci or so. However i was not able to trigger that check on its own.

The warning means that the right part is never null / undefined because 'string' + variable ?? 'fallback' reads as:

  1. 'string' + variable
  2. ?? Then check the result is null or undefined
  3. Then fallback to 'fallback'.

So just brackets are missing like 'string' + (variable ?? 'fallback').

(Otherwise you might get 'stringnull' as the result)

susnux avatar Jun 16 '24 14:06 susnux

@susnux The two issues i mentioned in person now on this PR after updating @nextcloud/dialogs

  • The file picker dialog loading fails (a quick patch for the dialogs library is below but I'm unsure if that is the proper approach)
  • Once that is patched the dialog does not return any selected file
diff --git a/lib/composables/isPublic.ts b/lib/composables/isPublic.ts
index ec89e8b..a1000ca 100644
--- a/lib/composables/isPublic.ts
+++ b/lib/composables/isPublic.ts
@@ -10,7 +10,7 @@ import { onBeforeMount, ref } from "vue"
 export const useIsPublic = () => {
        const checkIsPublic = () => (document.getElementById('isPublic') as HTMLInputElement|null)?.value === '1'

-       const isPublic = ref(true)
+       const isPublic = ref(checkIsPublic())
        onBeforeMount(() => { isPublic.value = checkIsPublic() })

        return {

juliusknorr avatar Jun 18 '24 09:06 juliusknorr