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.

Sign up for GitHub

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

Jump to bottom

feat(nuxt): enable config schema by default #19172

Merged
merged 15 commits into from Mar 4, 2023
Merged

feat(nuxt): enable config schema by default #19172

merged 15 commits into from Mar 4, 2023

Conversation

pi0
Copy link
Member

@pi0 pi0 commented Feb 20, 2023 •
edited by danielroe

🔗 Linked issue

#15592

❓ 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

Changes:

  • Enable schema without experimental flag to advocate using by module authors
  • Generate schema with nuxt prepare
  • Remove unnecessary markdown schema generation (integrations can still do the same!)
  • Always write .nuxt/schema directory to avoid confusion and removal
  • Extend AppConfig and AppConfigInput from internal _AppConfig to allow merging

📝 Checklist

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

@codesandbox
Copy link

codesandbox bot commented Feb 20, 2023

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

@pi0 pi0 marked this pull request as ready for review February 20, 2023 14:19
@pi0 pi0 requested review from Atinux and danielroe February 20, 2023 14:19
@danielroe danielroe added this to the v3.3 milestone Feb 20, 2023
Atinux
Atinux approved these changes Feb 21, 2023
View reviewed changes
Copy link
Member

@Atinux Atinux left a comment

Choose a reason for hiding this comment

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

Looks good to me at first sight, will have to try with nuxt-edge to see if it works properly on nuxt-icon for instance.

@danielroe
Copy link
Member

danielroe commented Feb 21, 2023 •
edited

Apologies for delay in reviewing properly. (Very much looking forward to this landing - and it does look great!) I'm planning to merge this in for v3.3 release (so ideally 1 Mar merge directly after patch release, then a week of testing on edge). Does that work for you @Atinux?

@pi0
Copy link
Member Author

pi0 commented Feb 21, 2023

@danielroe I think we should start thinking of semver/patch release branches this way we have more time for edge testing not blocked by patches :)

@danielroe
Copy link
Member

If you need more time to test, we can release the changes in next patch as long as we preserve default flag value as false.

@pi0
Copy link
Member Author

pi0 commented Feb 21, 2023

No i need to test the default behavior via edge channel on main branch :) Patch shouldn't be blocker will continue this on discord later today.

@pi0 pi0 changed the title feat: enable stable config schema fix: enable stable config schema Feb 21, 2023
@pi0
Copy link
Member Author

pi0 commented Feb 21, 2023 •
edited

Disabled flag for now to land into edge/patch without being blocked.

Checking (more generic) non blocker edge feat testing next week 👍🏼

@pi0 pi0 changed the title fix: enable stable config schema fix: config schema improvements Feb 21, 2023
@danielroe danielroe removed this from the v3.3 milestone Feb 21, 2023
@Atinux Atinux mentioned this pull request Feb 21, 2023
Merged
@danielroe danielroe added the 3.x label Feb 28, 2023
danielroe
danielroe reviewed Mar 3, 2023
View reviewed changes
packages/nuxt/src/core/schema.ts
Comment on lines +45 to +47
await nuxt.hooks.callHook('schema:beforeWrite', schema)
await writeSchema(schema)
await nuxt.hooks.callHook('schema:written')
Copy link
Member

Choose a reason for hiding this comment

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

would it make sense to move the hooks into writeSchema itself?

@danielroe danielroe changed the title fix: config schema improvements feat(nuxt): enable config schema by default Mar 3, 2023
@danielroe danielroe added this to the v3.3 milestone Mar 3, 2023
@danielroe danielroe merged commit 71225e5 into main Mar 4, 2023
@danielroe danielroe deleted the fix/stable-schema branch March 4, 2023 14:39
@danielroe danielroe mentioned this pull request Mar 8, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Atinux Atinux Atinux approved these changes

@danielroe danielroe danielroe approved these changes

Assignees
No one assigned
Labels
3.x 🍰 p2-nice-to-have
Projects
None yet
Milestone
v3.3
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@pi0 @danielroe @Atinux