Skip to content
This repository was archived by the owner on Apr 6, 2023. It is now read-only.
/frameworkPublic archive

feat(nuxt): config options for default keepalive, page & layout transitions - #5859

Merged
pi0 merged 20 commits into
mainfrom
feat/transition-config
Aug 23, 2022
Merged

feat(nuxt): config options for default keepalive, page & layout transitions#5859
pi0 merged 20 commits into
mainfrom
feat/transition-config

Conversation

@danielroe

Copy link
Copy Markdown
Member

🔗 Linked issue

resolvesnuxt/nuxt#14265

❓ Type of change

  • 📖 Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

This allows support for configuring page & layout transition default values, as well as keepalive. (Otherwise there is the tedious task of doing so for every individual file.)

In order to do this, we now inject #build/app.config.mjs to automatically expose all values within the app key in Nuxt config, rather than creating ever-more virtual files for particular use-cases. (We would, for example, have had to do this for layouts and page transitions separately otherwise.) This means we were able to drop meta.config.mjs and in general is a useful pattern for future app configuration.

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@danielroedanielroe added enhancement New feature or request 🍰 p2-nice-to-have Priority 2: nothing is broken but it's worth addressing labels Jul 12, 2022
@danielroe
danielroe requested a review from atinuxJuly 12, 2022 15:54
@danielroedanielroe self-assigned this Jul 12, 2022
@netlify

netlifyBot commented Jul 12, 2022

Copy link
Copy Markdown

Deploy Preview for nuxt3-docs canceled.

NameLink
🔨 Latest commita414fcd
🔍 Latest deploy loghttps://app.netlify.com/sites/nuxt3-docs/deploys/6304c375fa5ce1000818a2af

@danielroe
danielroe requested a review from pi0July 13, 2022 12:16
Comment threadpackages/nuxt/src/core/templates.ts Outdated
@pi0

pi0 commented Aug 22, 2022

Copy link
Copy Markdown
Member

Thanks for rebasing. Shall we type appConfig.nuxt from schema now?

@atinux

Copy link
Copy Markdown
Member

Beautiful work @danielroe ❤️

Shall we document more to use app.config.ts for such configurations from now on?

Comment threadpackages/schema/src/types/config.ts Outdated
@pi0

pi0 commented Aug 22, 2022

Copy link
Copy Markdown
Member

Shall we document more to use app.config.ts for such configurations from now on?

Currently, usage of useAppConfig().nuxt is kinda internal. For some, we might have built-time optimization opportunities (like not bundling transition bytes) while if we make users able to set via app.config, we cannot do this anymore as such config can be changed on runtime. I suggest not adding nuxt types from AppConfigInput as well for this PR (keep marking it as never for input but typing for AppConfig)

Comment threadpackages/schema/src/types/config.ts Outdated
pi0
pi0 approved these changes Aug 22, 2022
@pi0

pi0 commented Aug 22, 2022

Copy link
Copy Markdown
Member

(fixture seems broken)

@atinux

Copy link
Copy Markdown
Member

Could we take this opportunity to also give the possibility to set those values to false to disable keepalive, page & layout transitions?

@danielroe

danielroe commented Aug 23, 2022

Copy link
Copy Markdown
MemberAuthor

Indeed, that is one of the key use-cases of this PR. As implemented here, it is possible to set them to false and it will disable keepalive, page & layout transitions.

return _wrapIf(Transition, routeProps.route.meta.pageTransition ?? defaultPageTransition,
wrapInKeepAlive(routeProps.route.meta.keepalive, isNested && nuxtApp.isHydrating
// Include route children in parent suspense
return _wrapIf(Transition, routeProps.route.meta.pageTransition ?? appConfig._nuxt.pageTransition,

@pi0pi0Aug 23, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For this, we could have a bundler flag (like NUXT_PAGE_TRANSITION) that replaces value to false on build time otherwise setting app config to false won't be able to tree-shake... (but can be later. pointing to make clear about false handling with this PR)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yes. This PR does not enable tree-shaking out; it just sets default behaviour. We can't tree shake this way as user may wish to enable transitions/keepalive on a per-page basis.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I would love to be able to tree-shake out the Transition/Keepalive components but will it would need a different approach than this PR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's true. We use appConfig currently only to specify default but could be also used to specify feature flags. (swapping condition order when value is false to always disable transitions which I think would make more sense for false handling)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Maybe support an additional option, e.g. keepalive: 'disabled' as the user may simply wish to disable them by default but not for all routes?

@pi0

pi0 commented Aug 23, 2022

Copy link
Copy Markdown
Member

Thinking more about approaches, i think for specific nuxt bundled config, using an internal template with named exports (#build/nuxt.config.mjs ?) could be safer since we probably don't need nor could support HMR for these configs originating from nuxt.config and it makes it consistent with feature plans being able to treeshake at build time (appConfig is totally runtime-dynamic. we cannot depend on it for this unless passing another build flag)

@danielroe

Copy link
Copy Markdown
MemberAuthor

Okay - so just to confirm, we'll refactor back to original approach of this PR, but using nuxt.config.mjs filename?

@danielroe
danielroe requested a review from pi0August 23, 2022 12:22
pi0
pi0 approved these changes Aug 23, 2022

@pi0pi0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

@pi0pi0 changed the title feat(nuxt): config default keepalive, page & layout transitionsfeat(nuxt): config options for default keepalive, page & layout transitionsAug 23, 2022
@pi0
pi0 merged commit fc82b3b into mainAug 23, 2022
@pi0
pi0 deleted the feat/transition-config branch August 23, 2022 14:24
@pi0pi0 mentioned this pull request Aug 26, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

3.xenhancementNew feature or request🍰 p2-nice-to-havePriority 2: nothing is broken but it's worth addressing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

allow setting default route transition (page/layout) properties globally

3 participants

@danielroe@pi0@atinux