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

feat: add nextcloud-vue-import-transform script #5731

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Jun 21, 2024

☑️ Resolves

Imports from re-export are not optimal, having tree-shaking issues

import {
  NcButton as Button,
  useIsSmallMobile,
} as '@nextcloud/vue'

Individual imports are more optimal

import NcButton as Button from '@nextcloud/vue/dist/Components/NcButton.js'
import { useIsSmallMobile } from '@nextcloud/vue/dist/Composables/useIsMobile.js'

But changing it manually is painful for a huge app.

This script makes it automatically and tells if some imports cannot be processed.

Example

$ node ./scripts/nextcloud-vue-import-transform.mjs ../nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/ --dry

Transforming imports in /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src
👀 /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/app-menu.js [checked]
👀 /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/components/AppStoreBadge.vue [checked]
👀 /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/components/Card.vue [checked]
✅ /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/components/SlideShow.vue [transformed]
👀 /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/components/WizardPage.vue [checked]
👀 /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/components/pages/AboutNextcloud.vue [checked]
👀 /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/components/pages/DeviceIntegration.vue [checked]
👀 /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/components/pages/HubRelease.vue [checked]
👀 /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/components/pages/IntroAnimation.vue [checked]
✅ /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/components/pages/KeyNotes.vue [transformed]
✅ /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/components/pages/SharePage.vue [transformed]
👀 /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/components/pages/WhatsNew.vue [checked]
👀 /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/first-run.js [checked]
👀 /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/global-shim.d.ts [checked]
👀 /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/hub-release.ts [checked]
👀 /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/main.js [checked]
👀 /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/pages.ts [checked]
✅ /home/shgk/nextcloud/nextcloud-docker-dev/data/apps-extra/firstrunwizard/src/views/App.vue [transformed]

@ShGKme ShGKme requested review from susnux and Antreesy June 21, 2024 21:47
@ShGKme ShGKme self-assigned this Jun 21, 2024
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme force-pushed the feat/import-transform-script branch from 60eade1 to 0ec1c7c Compare June 21, 2024 21:47
@susnux
Copy link
Contributor

susnux commented Jun 22, 2024

Maybe it makes sense to have this in the eslint plugin or so?

@ShGKme
Copy link
Contributor Author

ShGKme commented Jun 22, 2024

Maybe it makes sense to have this in the eslint plugin or so?

Makes sense but I wanted to play with different apps to see if it actually makes result better, not worse.

@ShGKme ShGKme marked this pull request as draft June 22, 2024 19:31
@susnux
Copy link
Contributor

susnux commented Aug 30, 2024

Makes sense but I wanted to play with different apps to see if it actually makes result better, not worse.

As soon as you use NcRichText it makes no real difference, otherwise it reduces transformation time and output size.
Meaning for text app this does not help (10kB difference), but other work nicely.

But this is especially helpful for libraries that are later used by Webpack, like nc dialogs.


const MIXIN_NAMES = ['clickOutsideOptions', 'isFullscreen', 'isMobile', 'richEditor', 'userStatus']
const DIRECTIVE_NAMES = ['Focus', 'Linkify', 'Tooltip']
const modulesMap = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Any chance to update this map automatically?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants