Run check on all addons test #569
Conversation
|
commit: |
|
@manuel3108 this is the prototype for making check pass in all addons Edit: Wont need the tooling comment change Edit: The paraglide and Lucia hook are different. Would need to update paraglide to magic string implementation to match them. Running parseScript removes jsdoc comments from previous run. In this case paraglide comments were getting overridden when Lucia hook.server ran with ast. |
There was a problem hiding this comment.
Thanks for the PR btw, I will need to properly check everything once I have more time. But looks good and understandable overall.
The only thing i did not find on my inital pass through is to actually run pnpm check on the all-addons test. But maybe I missing something
|
|
||
| sv.file(`src/hooks.server.${ext}`, (content) => { | ||
| const { ast, generateCode } = parseScript(content); | ||
| js.imports.addNamespace(ast, '$lib/server/auth.js', 'auth'); | ||
| js.kit.addHooksHandle(ast, typescript, 'handleAuth', getAuthHandleContent()); |
There was a problem hiding this comment.
Can you please elaborate why we are not using this anymore? What was the problem with this call? Is it potentially fixable inside addHooksHandle or do we potentially have to check all other callees of this function for similar issues?
There was a problem hiding this comment.
Flow issue
-> paraglide adds doc comment to solve checking issues with handler types
-> adds to file correctly
-> Lucia parses script (*main issue here = this removes added doc comments as acorn doesn't support them, from my digging **note I've never used acorn)
-> Lucia adds it's doc comment but resulting file misses paraglide.
I went with the string route as the solution here as it also ties into Ben wanting to add more string returns in issue #557.
There was a problem hiding this comment.
I'm probably missing something, but I don't see any code adding a comment inside addHooksHandle that could cause this. Also tried running the adapter with JsDoc
There was a problem hiding this comment.
I don't think it's adding of the comment that fails. I think it is what it holds in memory as the ast (once again it's my first day with acorn and asts). I think it holds everything except the comments. Everytime I tried it with both using ast implementation it would drop 1 of the doc comments
There was a problem hiding this comment.
I will re look into it, I didn't document the why. From memory acorn doesn't read comments back in when it reads the same file back in. So first comment gets dropped when new hook is added, which is why second order has to be strong. But will need to reconfirm my human memory
There was a problem hiding this comment.
No problem. Acorn/sv is doing some weirdness around comments, but i did not experience anything like this. Maybe it's just esrap thats not printing them correctly or something.
Also please let me know, once you get back to it, about which comment you are talking exactly, as I didn't manage to figure that out.
|
Morning @manuel3108 Note: there hasn't been movement on storybook fix upstream yet. |
closes: #344
blocked by upstream pr on story book
Edit: storybookjs/storybook#31451