Skip to content

Type export path is broken on vue 9.0.0-rc0 #6870

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

Open
skjnldsv opened this issue May 2, 2025 · 18 comments · May be fixed by #6871
Open

Type export path is broken on vue 9.0.0-rc0 #6870

skjnldsv opened this issue May 2, 2025 · 18 comments · May be fixed by #6871
Assignees
Labels
2. developing Work in progress bug Something isn't working

Comments

@skjnldsv
Copy link
Contributor

skjnldsv commented May 2, 2025

Image

Image

I wanted to import ButtonType

cc @susnux @ShGKme

@skjnldsv skjnldsv added bug Something isn't working 1. to develop Accepted and waiting to be taken care of vue 3 labels May 2, 2025
@ShGKme ShGKme removed the vue 3 label May 2, 2025
@ShGKme ShGKme self-assigned this May 2, 2025
@ShGKme ShGKme added 2. developing Work in progress and removed 1. to develop Accepted and waiting to be taken care of labels May 2, 2025
@ShGKme
Copy link
Contributor

ShGKme commented May 2, 2025

The problem is that it is not reexported in components/index.ts.

A workaround that might work for you:

InstanceType<typeof NcButton>['$props']['variant']

@ShGKme
Copy link
Contributor

ShGKme commented May 2, 2025

I wanted to import ButtonType

If you actually mean ButtonType, and not ButtonVariant, you can use:

ButtonHTMLAttributes['type']

It's a built-in type (lib/DOM).

@ShGKme
Copy link
Contributor

ShGKme commented May 2, 2025

Or, if you need to use it for a component prop type, then import it from Vue:

import type { ButtonHTMLAttributes } from 'vue'

@ShGKme ShGKme linked a pull request May 2, 2025 that will close this issue
3 tasks
@dartcafe
Copy link
Contributor

dartcafe commented May 14, 2025

I think it is the same reason (9.0.0-rc.1):

https://github.com/nextcloud/polls/actions/runs/15010629163/job/42178558520?pr=4015

error during build:
src/components/Actions/modules/ActionAddOption.vue?vue&type=script&setup=true&lang.ts (9:19): 
"ButtonVariant" is not exported by "node_modules/@nextcloud/vue/dist/components/NcButton/index.mjs", 
imported by "src/components/Actions/modules/ActionAddOption.vue?vue&type=script&setup=true&lang.ts".

import NcButton, { ButtonVariant } from '@nextcloud/vue/components/NcButton'

I am not sure, if I understand the discussion right, is there no plan for a quick fix to be able to test the rc?

@ShGKme
Copy link
Contributor

ShGKme commented May 14, 2025

@dartcafe Do you mean the same that was discussed here?

@dartcafe
Copy link
Contributor

Yes. But I did not expect you remove the enum exports.

@ShGKme
Copy link
Contributor

ShGKme commented May 15, 2025

@dartcafe If there is a problem, could you explain it (in the context of the discussion in the PR)?

@susnux
Copy link
Contributor

susnux commented May 15, 2025

Probably in this case @dartcafe its a problem because of the missing type import:

- import NcButton, { ButtonVariant } from '@nextcloud/vue/components/NcButton'
+ import NcButton, { type ButtonVariant } from '@nextcloud/vue/components/NcButton'

@dartcafe
Copy link
Contributor

@dartcafe If there is a problem, could you explain it (in the context of the discussion in the PR)?

Isn't this issue about he missing type exports?

What can I more describe than I did before? --> #6870 (comment)

Importing of ButtonVariant isn't possible anymore since rc.0.

Probably in this case @dartcafe its a problem because of the missing type import:

  • import NcButton, { ButtonVariant } from '@nextcloud/vue/components/NcButton'
  • import NcButton, { type ButtonVariant } from '@nextcloud/vue/components/NcButton'

Unfortunately this does not work out of the box:

github/nextcloud/polls/src/components/Settings/AdminSettings/AdminJobs.vue
8  |  import { t } from '@nextcloud/l10n'
9  |  
10 |  import NcButton, { type ButtonVariant } from '@nextcloud/vue/components/NcButton'
   |                          ^
11 |  
12 |  import { AdminAPI } from '../../../Api/index.ts'
file: github/nextcloud/polls/src/components/Settings/AdminSettings/AdminJobs.vue:5:24
    at constructor (github/nextcloud/polls/node_modules/@babel/parser/lib/index.js:360:19)
    at Parser.raise (github/nextcloud/polls/node_modules/@babel/parser/lib/index.js:3338:19)
    at Parser.unexpected (github/nextcloud/polls/node_modules/@babel/parser/lib/index.js:3358:16)
    at Parser.expect (github/nextcloud/polls/node_modules/@babel/parser/lib/index.js:3668:12)
    at Parser.parseNamedImportSpecifiers (github/nextcloud/polls/node_modules/@babel/parser/lib/index.js:14055:14)
    at Parser.parseImportSpecifiersAndAfter (github/nextcloud/polls/node_modules/@babel/parser/lib/index.js:13902:37)
    at Parser.parseImport (github/nextcloud/polls/node_modules/@babel/parser/lib/index.js:13895:17)
    at Parser.parseStatementContent (github/nextcloud/polls/node_modules/@babel/parser/lib/index.js:12540:27)
    at Parser.parseStatementLike (github/nextcloud/polls/node_modules/@babel/parser/lib/index.js:12432:17)
    at Parser.parseModuleItem (github/nextcloud/polls/node_modules/@babel/parser/lib/index.js:12409:17)

@ShGKme
Copy link
Contributor

ShGKme commented May 15, 2025

@dartcafe I don't understand what exact problem you are trying to solve.

If you want to have IDE support with autocomplete and check for the NcButton's variant - it should work with the plain string thanks to TypeScript.

If you need a type of the variant to use it somewhere else, where you, for example, extend a button, or use NcButton's props in data — you can use InstanceType<typeof NcButton>['$props']['variant'] or vue-component-type-helpers lib, or import the type with the fix in the linked PR.

If you want to import an object with all the possible variants (for example, to iterate over them) - this is not possible now. In your PR you said that you needed it for the list of possible values, but it is covered by IDE and TypeScript as was said in the PR.

@dartcafe
Copy link
Contributor

What problem: I assumed, this issue is about the broken type export and I assumed that this will be fixed, too.

How was the journey:

  • A while ago while migrating to the new lib (some alpha) the type for NcButton changed and expected the type "ButtonType" (ts error message I assume this commit).
    So I imported it and replaced all occurrences for the ButtonType with the imported type.

  • Then currently the exported ButtonType became the ButtonVariant. I changed all occurencies with not much effort.

  • With rc.0 the export was broken (judging from the topic of this issue) and now I am just reporting, that it is still broken in rc.1.

And I sit here and wonder what happens.

  • Is the enum export removed by intention?
  • Will the enum export come back?
  • Is it a bug, which will get fixed?
  • Will it finally stay removed?

I am searching for clarification. If you say, it won't come back: OK, I can live with it, although I prefer using the enums if available. But I do not want to change back and forth again.

@ShGKme
Copy link
Contributor

ShGKme commented May 16, 2025

What problem: I assumed, this issue is about the broken type export and I assumed that this will be fixed, too.

Yes, It will (there is a fixing PR).

But could you clarify how you plan to use this type? What problem are you solving with the type?

  • Is the enum export removed by intention?

Yes:

  • ENUMs are not a part of JS, and also not a part of static type analyzing
    • It is not supported in a kind of modern TS usage, such as Node.js environment
    • It is one of a few old TS features, when TS was used as a programming language and transpiler, rather than a static type analyzer (linter)
    • The only other such (still used) feature is decorators, and it's already conflicting with the ES decorators proposal, making problems for the ecosystem. TS now has 2 decorators implementation. And will probably have 2 ENUM implementations when the ES ENUM proposal is on stage 3.
  • In this specific case there is no need for a nominal type. It requires additional import for each NcButton usage, and additional
  • Will the enum export come back?

No

  • Is it a bug, which will get fixed?

No

  • Will it finally stay removed?

Yes

@dartcafe
Copy link
Contributor

dartcafe commented May 16, 2025

But could you clarify how you plan to use this type? What problem are you solving with the type?

I don't want to solve any problem with enum, I wanted to solve the problem of confusion. As I explained before:

  • First it was required on introduction,
  • then it got renamed and
  • then removed.

And now I simply wanted to know, what the plan about this. I assumed it was removed unintentionally, but it was obviously not.

But you answered my question: No more usage of the enums planned.

@dartcafe

This comment has been minimized.

@dartcafe

This comment has been minimized.

@ShGKme
Copy link
Contributor

ShGKme commented May 16, 2025

If you are using TS and <script setup>, you can simplify props definition to:

const { buttonVariant = 'primary' } = defineProps<{
  buttonVariant?: ButtonVariant
}>()
``

@dartcafe

This comment has been minimized.

@susnux
Copy link
Contributor

susnux commented May 26, 2025

I tested this now and it works if:

import type { ButtonVariant } from '@nextcloud/vue/components/NcButton'

it does not (as expected) by:

import type { ButtonVariant } from '@nextcloud/vue'

So the question / options:

  • always export types also from main entry point (like the PR of @ShGKme does)
  • types should be only exported from component module (main entry only exports values)

I personally would say having types in the main entry point will maybe become a mess so we should only export from component module. But I am open for other opinions :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants