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

fix(common): clean up types #73

Merged
merged 2 commits into from
Jul 25, 2024
Merged

fix(common): clean up types #73

merged 2 commits into from
Jul 25, 2024

Conversation

eunjae-lee
Copy link
Contributor

What?

This PR does several things:

  • move AppConfig patch to index.d.ts, so that all the projects that use nuxt-base will have the same type.
  • configure ./types to be auto-imported
  • remove unnecessary type imports

Comment on lines +10 to +21
interface AppConfigInput {
type: PluginType;
appBridge?: AppBridgeConfig;
auth?: AuthConfig;
}

interface AppConfig {
type: PluginType;
appBridge: AppBridgeConfig;
auth: AuthConfig;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've discovered that we can have different AppConfigInput and AppConfig (output):

https://nuxt.com/docs/guide/directory-structure/app-config#typing-app-config

So we can have appBridge and auth being optional, but in the end, Nuxt will merge all the app configs from the layers, and provide the complete version of AppConfig.

Copy link
Contributor

Choose a reason for hiding this comment

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

that makes sense maybe adding a comment would help us understand this better in future? WDYT?

space-plugins/nuxt-starter/app.config.ts Show resolved Hide resolved
Comment on lines 7 to 8
},
auth: {
endpointPrefix: '/api/connect',
initOauthFlowUrl: `/api/connect/storyblok`,
successCallback: '/',
errorCallback: '/401',
},
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now we no longer need to have full appConfig. Only things that we want to override.

@eunjae-lee eunjae-lee requested a review from BibiSebi July 24, 2024 14:23
Copy link
Contributor

@BibiSebi BibiSebi left a comment

Choose a reason for hiding this comment

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

Looking great 💪 thank you for the speedy adjustments 🙏

Left one small comment.

@eunjae-lee eunjae-lee merged commit 08d3b69 into main Jul 25, 2024
1 check passed
@eunjae-lee eunjae-lee deleted the fix/clean-up-types branch July 25, 2024 08:02
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.

2 participants