First implementation #8
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
thlehmann-ionos
force-pushed
the
tl/dev/webpack-vue-2-typescript
branch
6 times, most recently
from
902fce3 to
f75e888
Compare
thlehmann-ionos
force-pushed
the
tl/dev/webpack-vue-2-typescript
branch
from
f75e888 to
08838f9
Compare
|
printminion-co
requested changes
Jul 2, 2024
There was a problem hiding this comment.
- lets update packages
- npx npm-check -u
- let's commit the generated js in order to be able to test the app
- after Device session creation the input field should be set to empty
- main.js cache busting?
- de.js cache busting?
- sourcemaps
- do we need it in production?
- lets use relative url (see comments in commits)
- I would add an integration test to see if the code works with
stable29. a-lá https://github.com/IONOS-Productivity/nc-googleanalytics/blob/b883fb78b22c31e55d586723cff32874d7fc18f0/tests/Integration/GoogleAnalyticsTest.php
thlehmann-ionos
force-pushed
the
tl/dev/webpack-vue-2-typescript
branch
from
08838f9 to
e56a5e9
Compare
|
Package updates, commit reorder and squash done. |
|
Not otherwise addressed/resolved feedback from other conversations:
Done.
As discussed: no.
I can't find how-to's / best practices in Nextclouds App dev guides and it looks like cache-busting is not in place for viewer-main (viewer app). Let's research this in another follow-up issue and let's discuss with Nextcloud.
I'll take a look at it. |
thlehmann-ionos
force-pushed
the
tl/dev/webpack-vue-2-typescript
branch
from
e56a5e9 to
340239f
Compare
Signed-off-by: Thomas Lehmann <[email protected]>
- @nextcloud/stylelint-config was not directly referenced and doesn't seem to be a peer dependency either - eslint added - stylelint added - missing peer dependencies of @nextcloud/eslint-config added - a couple of other dependencies added Checked via depcheck and npm-check. One false report of "vue-eslint-parser" not being installed followed by an npm registry error was ignored. Signed-off-by: Thomas Lehmann <[email protected]>
* Add tsconfig.json (from Nextcloud, with some modifications)
* "include" patterns adapted
* "jest" removed from types for now (no tests yet)
* types reduced to ["vue"]
* vueCompilerOptions omitted
* removing it does not change the outcome
* guides on Webpack + Vue + Typescript don't show it
* probably only necessary for the VScode extension "Volar" [1]
* ts-node omitted
* we're not compiling for node environments
* set allowImportingTsExtensions and noEmit false
To prevent
ERROR in ./src/main.ts
Module build failed (from ./node_modules/ts-loader/index.js):
Error: TypeScript emitted no output for /app/src/main.ts.
at makeSourceMapAndFinish (/app/node_modules/ts-loader/dist/index.js:55:18)
at successLoader (/app/node_modules/ts-loader/dist/index.js:42:5)
at Object.loader (/app/node_modules/ts-loader/dist/index.js:23:5)
A helpful resource was [2]
* webpack.js: added appendTsSuffixTo option
To tell Webpack to pass .vue thorugh the Typescript compiler too
* Add shims-vue.d.ts with re-export of Vue types
* Installed @vue/tsconfig because it's referenced by tsconfig.json
* Adapted filename pattern for entrypoint to .ts
* Added type declaration for "n" and "t" because in the Javascript they
were referenced as globals (this should later be changed to some
proper import of a typedef)
* main.ts: l10n was added to explicitly pass typed references, not rely
on globals
[1]: https://v2.vuejs.org/v2/guide/migration-vue-2-7#Volar-Compatibility
[2]: https://github.com/nextcloud/cookbook/pull/2059/files?diff=unified&w=1#diff-b55cdbef4907b7045f32cc5360d48d262cca5f94062e353089f189f4460039e0
Signed-off-by: Thomas Lehmann <[email protected]>
Add @ts-expect-error to imports to fix tsc complaining about the module not exporting type definitions The reported error would be: @ts-expect-error: Cannot find module or its corresponding type declarations Signed-off-by: Thomas Lehmann <[email protected]>
1. Copy files cp ../../apps/settings/src/components/AuthToken* src/components/ cp ../../apps/settings/src/logger.ts src/logger.ts cp ../../apps/settings/src/store/authtoken.ts src/store/authtoken.ts 2. Install dependencies Signed-off-by: Thomas Lehmann <[email protected]>
Signed-off-by: Thomas Lehmann <[email protected]>
In URLs and translation namespaces Signed-off-by: Thomas Lehmann <[email protected]>
* package.json: add Nextcloud dependencies (from Nextcloud)
* main.ts: register pinia
Otherwise the following error would be thrown:
TypeError: Cannot read properties of undefined (reading '_s')
at i (pinia.mjs:1714:20)
at setup (AuthTokenList.vue:11:32)
at mn (vue.runtime.esm.js:3033:30)
at vue.runtime.esm.js:2457:27
...
* remove .ts suffix from import paths (causes build error otherwise)
* add declare global to define Nextcloud types. This should be changed
to @nextcloud/typings.
* annotated import of Nextcloud components (JS) with @ts-expect-error
to solve tsc errors. This is most likely due to missing type
definitions. Unclear how Nextcloud core made this working.
* the eslint rule set @nextcloud/eslint-config/typescript was added to
prevent import path errors
The build fails with errors like
ERROR in /app/src/components/AuthTokenSetup.vue.ts
47:9-14
[tsl] ERROR in /app/src/components/AuthTokenSetup.vue.ts(47,10)
TS2339: Property 'reset' does not exist on type 'CreateComponentPublicInstance<{}, { authTokenStore: Store<"auth-token", { tokens: IToken[]; }, {}, { updateToken(token: IToken): Promise<any>; addToken(name: string): Promise<ITokenResponse | null>; deleteToken(token: IToken): Promise<...>; wipeToken(token: IToken): Promise<...>; renameToken(token: IToken, newName: ...'.
This hints at Typescript not being able to infer types from the
component definition.
Related issues:
* vuejs/vue#9873
* vuejs/vue#12628
* vuejs/vue#8721
Unsuccessfully attempted proposed solutions:
* Declaring return types on methods like:
reset(): void {
async submit(): Promise<void> {
* Using arrow functions
Did not work and would introduce new errors anyway
* Defining interfaces for the returned component
Caused another rabbit hole of more and more required type
definitions, that should be infered by the Vue typings anyway
Signed-off-by: Thomas Lehmann <[email protected]>
…ill fails with one kind of TS error)
Merely moving the property after another fixes a Typescript error. I'm
baffled.
This fixes one of three Vue/Typescript errors:
ERROR in /app/src/components/AuthTokenSetup.vue.ts
47:9-14
[tsl] ERROR in /app/src/components/AuthTokenSetup.vue.ts(47,10)
TS2339: Property 'reset' does not exist on type 'CreateComponentPublicInstance<{}, { authTokenStore: Store<"auth-token", { tokens: IToken[]; }, {}, { updateToken(token: IToken): Promise<any>; addToken(name: string): Promise<ITokenResponse | null>; deleteToken(token: IToken): Promise<...>; wipeToken(token: IToken): Promise<...>; renameToken(token: IToken, newName: ...'.
Still fails with two other errors where Typescript can't infer types on
properties referenced via this.$refs.
Related:
* vuejs/vue#12628 (comment)
Signed-off-by: Thomas Lehmann <[email protected]>
It conflicts with a necessary workaround for a tsc bug fixed in the previous commit. Signed-off-by: Thomas Lehmann <[email protected]>
This works around build errors like this:
ERROR in /app/src/components/AuthTokenSetupDialog.vue.ts
93:29-35
[tsl] ERROR in /app/src/components/AuthTokenSetupDialog.vue.ts(93,30)
TS2339: Property 'select' does not exist on type 'ComponentPublicInstance<{}, {}, {}, {}, {}, {}, {}, {}, false, ComponentOptionsBase<any, any, any, any, any, any, any, any, any, any>> | Element | Vue<...> | (ComponentPublicInstance<...> | ... 1 more ... | Vue<...>)[]'.
Property 'select' does not exist on type 'ComponentPublicInstance<{}, {}, {}, {}, {}, {}, {}, {}, false, ComponentOptionsBase<any, any, any, any, any, any, any, any, any, any>>'.
This is actually not a proper fix. It would be better to declare type
definitions once for the NcTextField using Typescript's
declare module '.../NcTextField.mjs' {
}
Signed-off-by: Thomas Lehmann <[email protected]>
Signed-off-by: Thomas Lehmann <[email protected]>
Signed-off-by: Thomas Lehmann <[email protected]>
* Route name in routes.php adapted * API URL changed in authtoken.ts Signed-off-by: Thomas Lehmann <[email protected]>
Signed-off-by: Thomas Lehmann <[email protected]>
https://eslint.org/docs/latest/rules/quotes#single Signed-off-by: Thomas Lehmann <[email protected]>
Copy from Nextcloud's /apps/settings with slight modifications Signed-off-by: Thomas Lehmann <[email protected]>
There's no reason to keep the name. Signed-off-by: Thomas Lehmann <[email protected]>
Signed-off-by: Thomas Lehmann <[email protected]>
Signed-off-by: Thomas Lehmann <[email protected]>
Just to test the process. Signed-off-by: Thomas Lehmann <[email protected]>
In order to be able to run all tasks in one place Co-Authored-By: Thomas Lehmann <[email protected]> Signed-off-by: Thomas Lehmann <[email protected]>
in order to avoid
"./composer.json" does not match the expected JSON schema:
- authors[0].homepage : Invalid URL format
Signed-off-by: Thomas Lehmann <[email protected]>
After running $ composer install --prefer-dist Signed-off-by: Thomas Lehmann <[email protected]>
Signed-off-by: Thomas Lehmann <[email protected]>
We don't expose API endpoints Signed-off-by: Thomas Lehmann <[email protected]>
Test (via unit tests) whether it's still compatible with the upcoming stable. Signed-off-by: Thomas Lehmann <[email protected]>
Signed-off-by: Thomas Lehmann <[email protected]>
Navigation removed to remove the navigation entry from the header. A redirect config redirects from /apps/settings to /apps/simplesettings. Signed-off-by: Thomas Lehmann <[email protected]>
This is what we'll use, should suffice for us. Signed-off-by: Thomas Lehmann <[email protected]>
* Checked with npm-check * Ran make build; lint and build OK, manual tests OK
thlehmann-ionos
force-pushed
the
tl/dev/webpack-vue-2-typescript
branch
from
1a5fad8 to
f331537
Compare
|
Final npm update of minor versions, missing |
Git workflow failes because @nextcloud/eslint-config's peer dependencies requires 13.x Signed-off-by: Thomas Lehmann <[email protected]>
thlehmann-ionos
force-pushed
the
tl/dev/webpack-vue-2-typescript
branch
from
f331537 to
637ec91
Compare
It's referenced by stylelint.config.js via "extends". Although stylelint does not fail in the dev container, the Github workflow failed without it. Signed-off-by: Thomas Lehmann <[email protected]>
thlehmann-ionos
force-pushed
the
tl/dev/webpack-vue-2-typescript
branch
from
637ec91 to
3ce55b8
Compare
thlehmann-ionos
force-pushed
the
tl/dev/webpack-vue-2-typescript
branch
from
3ce55b8 to
851c693
Compare
|
Removed dummy commit to make workflows runnable on this dev branch. Most workflows were OK, two or three could not start because no workers available |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First steps for simplified settings.
Simplesettings:
Approach:
apps/settingsandapps/filesDifferences: