Ensure that the import resolver respects installOptions.externalPackage. #1070
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/pikapkg/snowpack/itj0r5ack |
| @@ -56,7 +56,7 @@ export function createImportResolver({ | |||
| config, | |||
| }: ImportResolverOptions) { | |||
| return function importResolver(spec: string): string | false { | |||
| if (URL_HAS_PROTOCOL_REGEX.test(spec)) { | |||
| if (URL_HAS_PROTOCOL_REGEX.test(spec) || config.installOptions.externalPackage?.includes(spec)) { | |||
There was a problem hiding this comment.
ha, this looks like an outdated version of what we use in build.ts. Can you replace both here and build.ts with:
// Ignore "http://*" imports
if (url.parse(spec).protocol) {
return spec;
}
// Ignore packages marked as external
if (this.config.installOptions.externalPackage?.includes(spec)) {
return spec;
}
It's definitely a little more verbose, but these functions are notoriously buggy so good documentation is important.
There was a problem hiding this comment.
Sure thing, done!
|
I feel that the added test doesn't cover your use case. |
|
Hmm, you're right. I'm not sure what happened -- it was definitely failing when I first created the PR. I'll dig into it. |
|
So it turns out that while the build (and dev) log an error, the error is actually ignored and the build completes "successfully" making my test a no-op. Is there some way to test Snowpack's console output for errors? If not, I'm not sure how to make a test for this PR, since in the end it only affects non-fatal error output. |
|
I actually believe that this was fixed by #1072, I can't reproduce when rebasing off of master now that errors always propagate up to the build. I'll merge onto master to confirm, if there is still an error here then it's in the test runner, and not this test itself |
This makes the dev import resolver pass through any imported packages that are on the
installOptions.externalPackagelist. Fixes #1067.Testing
I added a test to ensure that building respects external packages. The option also affects the output of dev mode but I'm not sure how to test that -- could you point me to an example?
This change is