feat: tolerate module resolution errors on startup #1529
Conversation
✅ Deploy Preview for creative-fairy-df92c4 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| if (details && details.importer.startsWith('/@fs/')) { | ||
| wxt.logger.error(err.message); | ||
| wxt.logger.info('Waiting for import specifier to be fixed...'); | ||
| watchFile = details.importer.slice(5); |
There was a problem hiding this comment.
This isn't perfect, since the broken import may not be the fault of the importer. For example, it could be a configuration issue with tsconfig.json or something.
But it's not a big deal, IMO, since the developer can restart the dev process or save the importer's file if necessary.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
aleclarson
force-pushed
the
feat/module-resolution-errors-1
branch
2 times, most recently
from
a5b3520 to
9639e7f
Compare
This comment was marked as resolved.
This comment was marked as resolved.
|
Okay, should be ready for review! @aklinker1 |
There was a problem hiding this comment.
I like these changes, but we need to update the tests in detect-dev-changes.test.ts. It's a very well tested function with 100% coverage. So we need to add coverage for the two new cases: no build output and error files.
| return error.plugin === 'vite:load-fallback' && error.hook === 'load'; | ||
| } | ||
|
|
||
| export function resolveErrorFiles(err: Error): string[] { |
There was a problem hiding this comment.
And we should definintely add tests for this function. Will be nice to have example errors we can refer to to understand what exactly what is happening better.
aleclarson
force-pushed
the
feat/module-resolution-errors-1
branch
from
39d22ad to
6c969eb
Compare
|
@aklinker1 I updated existing tests for
Do you mean unit tests with dummy errors resembling real ones, or are you asking for integration tests that reproduce the errors using Babel, Rollup, Vite, etc? |
I was just taking about unit tests, but integration tests would probably be better for this. |
Overview
Avoid crashing the dev server if a broken import exists.
Unrelated to this PR: You can see the suggestion to install
vite-tsconfig-paths, but as I understand it, WXT doesn't recommend using that package; preferring that the user setsaliasinwxt.config.tsinstead.Manual Testing
importstatement