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

feat(content): add spaLoadingTemplate to content resolution #907

Merged
merged 3 commits into from
Oct 28, 2024

Conversation

nikolasdas
Copy link
Contributor

πŸ”— Linked issue

resolves #906

❓ 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)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This simply adds spa-loading-template.html to the default content config and updates the docs accordingly

Copy link

netlify bot commented Oct 18, 2024

βœ… Deploy Preview for nuxt-tailwindcss ready!

Name Link
πŸ”¨ Latest commit 609ed13
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt-tailwindcss/deploys/671fcf6a3b68f70008316ece
😎 Deploy Preview https://deploy-preview-907--nuxt-tailwindcss.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

what-the-diff bot commented Oct 18, 2024

PR Summary

  • Added a new template to the configuration
    The team included a new file named spa-loading-template.html within the content paths specified in our Tailwind configuration.

  • Updated Resolver Logic
    The system used for fetching templates was updated. It will now include the newly added spa-loading-template.html.

These changes ensure that the new template can be easily identified and utilized by our system, improving the product's flexibility and usability.

src/resolvers.ts Outdated
@@ -53,6 +53,7 @@ const resolveContentPaths = (srcDir: string, nuxtOptions = useNuxt().options) =>
r(`{A,a}pp${sfcExtensions}`),
r(`{E,e}rror${sfcExtensions}`),
r(`app.config${defaultExtensions}`),
r(`spa-loading-template.html`),
Copy link
Collaborator

Choose a reason for hiding this comment

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

how about adding this only when nuxtOptions.ssr === false?

Copy link
Collaborator

Choose a reason for hiding this comment

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

and spaLoadingTemplate !== false

Copy link
Collaborator

Choose a reason for hiding this comment

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

but does the HTML include nuxt.options.css in head?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how about adding this only when nuxtOptions.ssr === false?

Wouldn't nuxtOptions.ssr === false mean it is only included when ssr is turned off for the entire application?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and spaLoadingTemplate !== false

I guess a check for nuxtOptions.spaLoadingTemplate !== false should work, yes. I thought since error.vue e.g. is included without checking if the file exists in the project that a simple list of all possible files is preferred, otherwise I can add this check :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

So nuxtOptions.spaLoadingTemplate can be a string as well i.e., a custom name for spa-loading-template.html possibly - but it will be applied when ssr: false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but does the HTML include nuxt.options.css in head?

Yes, the generated html includes the tailwind css bundle

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, I didn't realize a custom file name could be passed... I will check this and the ssr option asap and report back

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'm a little bit confused. When I turn off ssr for the entire application, the styles do not get included, but when I use routeRules the styles get injected. I've only tested this in the playground for now.

Copy link
Collaborator

@ineshbose ineshbose Oct 22, 2024

Choose a reason for hiding this comment

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

Just pushed - seems to be working OK; will need to check the appDir! (we'll need to write tests)

@ineshbose ineshbose changed the title feat: add spa-loading-template.html to content config feat(content): add spaLoadingTemplate to content resolution Oct 28, 2024
Copy link
Collaborator

@ineshbose ineshbose left a comment

Choose a reason for hiding this comment

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

Nice addition, thanks ❀️

@ineshbose ineshbose merged commit b54746e into nuxt-modules:main Oct 28, 2024
7 checks passed
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.

Add spa-loading-template.html to content config
2 participants