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: move ssr script injection into plugin for nitro compat #86

Merged
merged 1 commit into from
May 24, 2021

Conversation

danielroe
Copy link
Collaborator

@danielroe danielroe commented May 21, 2021

  • spa script injected with module hook
  • server script injected with plugin

tested with vite, nitro, server, spa, and ssr (with spa fallback)

cc: @atinux

* spa script injected with module hook
* server script injected with plugin
@codecov
Copy link

codecov bot commented May 21, 2021

Codecov Report

Merging #86 (5d1a543) into master (c34b0a3) will decrease coverage by 22.72%.
The diff coverage is 44.44%.

Impacted file tree graph

@@             Coverage Diff              @@
##            master      #86       +/-   ##
============================================
- Coverage   100.00%   77.27%   -22.73%     
============================================
  Files            2        2               
  Lines           19       22        +3     
  Branches         2        2               
============================================
- Hits            19       17        -2     
- Misses           0        4        +4     
- Partials         0        1        +1     
Impacted Files Coverage Δ
lib/module.js 66.66% <44.44%> (-33.34%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c34b0a3...5d1a543. Read the comment docs.

@pi0
Copy link
Contributor

pi0 commented May 24, 2021

@danielroe Have you also tried ssr: false > nuxt start? (if is broken, I guess previous/current implementation is also broken...)

@danielroe
Copy link
Collaborator Author

ssr: false > nuxt start works with color-mode in modules (rather than buildModules) which is also the behaviour in current implementation.

An alternative approach would be to modify .nuxt/views/app.template.html and not use vue-meta at all - what do you think?

@pi0
Copy link
Contributor

pi0 commented May 24, 2021

An alternative approach would be to modify .nuxt/views/app.template.html and not use vue-meta at all - what do you think?

Thinking of same path (we may also try nuxt hook to modify APP in context). Mentioned this because @nuxt/pwa is also having same issue. (anyway it is off this PR topic. Will message you ;D)

@atinux atinux merged commit 9002bb0 into nuxt-modules:master May 24, 2021
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.

3 participants