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: reload controllers on change during development #76

Closed
wants to merge 1 commit into from

Conversation

rchl
Copy link
Collaborator

@rchl rchl commented Mar 15, 2022

Add a watcher so that whole Nuxt is reloaded on changing API routes and changes are applied at runtime.

Also switched to Nuxt's own resolveAlias helper instead of custom solution. This is not strictly related to the fix though.

Fixes #16

@rchl rchl requested a review from ezypeeze March 15, 2022 21:43
@codecov
Copy link

codecov bot commented Mar 15, 2022

Codecov Report

Merging #76 (893855d) into master (33742b8) will decrease coverage by 0.21%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #76      +/-   ##
==========================================
- Coverage   92.22%   92.01%   -0.22%     
==========================================
  Files           5        5              
  Lines         579      576       -3     
==========================================
- Hits          534      530       -4     
- Misses         45       46       +1     
Impacted Files Coverage Δ
lib/module.js 81.39% <100.00%> (ø)
lib/server_middleware/api.js 94.97% <100.00%> (+0.02%) ⬆️
lib/utility/controllers.js 93.90% <100.00%> (-0.50%) ⬇️

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 33742b8...893855d. Read the comment docs.

@ezypeeze
Copy link
Owner

Codecov is failed for some reason

@rchl
Copy link
Collaborator Author

rchl commented Mar 17, 2022

Not sure what is that about. 3 lines or so lost coverage but it doesn't seem very relevant. Unless the Nuxt reload triggers during testing and somehow messes with coverage but then I would expect a lot more misses.

Also note that this fix is not ideal as changes in imports that the controllers depend on won't trigger reload thus those won't be reflected. But it's still somewhat of an improvement for DX.

@ezypeeze
Copy link
Owner

Not sure what is that about. 3 lines or so lost coverage but it doesn't seem very relevant. Unless the Nuxt reload triggers during testing and somehow messes with coverage but then I would expect a lot more misses.

Also note that this fix is not ideal as changes in imports that the controllers depend on won't trigger reload thus those won't be reflected. But it's still somewhat of an improvement for DX.

You want to merge it anyway? I can bypass the checks failing and still merge. Let me know

@rchl
Copy link
Collaborator Author

rchl commented Mar 18, 2022

I'm a bit on the fence. It semi-fixes the issue. Ideally it should be fixed completely but I'm not sure how. I think the imports would have to be static for it to work automatically but that means that those would have to be pre-generated before Nuxt is started or something...

I'm fine with not merging this or leaving it open until I have more time to investigate.

@pi0 suggested that we talk about solving this for Nitro but not sure in what form the discussion should be.
Also, I'd really would like to improve this for Nuxt 2 since it might be a while until I can use Nitro (just tried today and it's still problematic for various reasons).

@rchl rchl marked this pull request as draft March 21, 2022 08:28
@rchl rchl closed this Sep 3, 2022
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.

Modifying a controller doesn't reload it
2 participants