fix(sass-imports): resolve module imports relative to file being processed #478
Conversation
…essed
Currently sass-imports that use the module import syntax (eg. `~module-to-import/some.sass`) are resolved relative to the vue-jest package. If you only have one version of that package, and everything has been hoisted to the same level as vue-jest (or higher), then things work as expected.
Where things start to fail is when you have more than one instance of the sass package you are trying to import as it is possible the wrong version will be loaded, and it is concievable that the file won't be found.
Where I encountered this problem, we had something like the following:
```
node_modules/
component-library/
node_modules/
sass-library@3.0.0
mixins.sass
some-component.vue:
<style lang="sass">
@import '~sass-library/mixins.scss';
class foo {
@use someMixinFromV3
}
</style>
sass-library@1.0.0
mixins.sass
@vue/vue2-jest
lib
module-name-mapper-helper.js
```
Notice that "sass-library" v1 was hoisted to the top level, but "component-library" depends on v3 of sass-library so v3 is in a nested node_modules directory.
When "@vue/vue2-jest" does `require.resolve('sass-library/mixins.scss')`, while processing `component-library/some-component.vue`, the module search path is relative to the vue2-jest source file and not relative to some-component.vue like you would expect. The end result is that it finds the top-level `sass-library@1.0.0` and not the v3 version.
To fix this, I am now passing in the optional "paths" value to require.resolve (available since node 8.9 https://nodejs.org/api/modules.html#requireresolverequest-options) and providing the path to the source file being processed instead. This will start the module resolution search at some-component.vue and the nested `node_modules/sass-library`` v3 will be found instead.
|
@lmiller1990 is this something that can be looked into / reviewed by you? Let me know if there is a different process in place for reviews in this project, I just saw that you were the most active contributor recently. |
|
Hi @thebanjomatic! Sorry for the lack of a review. I am the most active on this project, although I am not using Jest much nowadays, and most of my time is consumed on other projects. We definitely need a more active maintainer. Either way, I have commit and release privileges, so happy to merge and release this as long as nothing is broken. Any chance we can add a test? Be much more comfortable merging and release a new version if we have a test for the new code you've added. There's a bunch of example projects here: https://github.com/vuejs/vue-jest/tree/master/e2e, maybe you can add something there? Ideally a reproduction of your exact problem, so we know this solves it. |
|
@lmiller1990 thanks I will look into adding an e2e test. I'll have to think about how to reproduce this issue in a minimal way since it currently involves a minimum of 3 packages (one of which is being included twice). |
thebanjomatic
force-pushed
the
fix/resolve-path
branch
from
932c9bc
to
a7d6851
Compare
|
I added e2e tests for vue2 and vue3, it was a little messy due to the required setup, but I was able to use the file protocol to allow multiple versions of the same local package, but I then needed to also exclude them from the workspace since you can't have multiple packages in a yarn workspace with the same package name. While the tests work, I realized as I was writing this that I probably don't need to reproduce the multiple versions of a package specifics, but I could have instead just used no-hoist to ensure that the sass library isn't hoisted, and that would have also failed to resolve the If you'd like me to go back and simplify the tests, let me know and I can do that. |
|
Seems fine, if CI is green I'm happy to ship this. It's running now. |
|
Is there a policy on backporting to the 27.x release also? |
|
There's no such policy or process. You'd need to open a new PR against 27.x. Supporting old versions is not super ideal - are you unable to upgrade to Jest 28? |
|
Hello, I release 28.0.1 so you can have the fix: https://github.com/vuejs/vue-jest/releases/tag/v28.0.1 We can back-release 27.x if you'd like to make a PR. |
Currently Sass imports that use the module import syntax (eg.
~module-to-import/some.sass) are resolved relative to the vue-jest package. If you only have one version of that package, and everything has been hoisted to the same level as vue-jest (or higher), then things work as expected.Where things start to fail is when you have more than one instance of the sass package you are trying to import as it is possible the wrong version will be loaded, and it is concievable that the file won't be found.
Where I encountered this problem, we had something like the following:
Notice that "sass-library" v1 was hoisted to the top level, but "component-library" depends on v3 of sass-library so v3 is in a nested node_modules directory.
When "@vue/vue2-jest" does
require.resolve('sass-library/mixins.scss'), while processingcomponent-library/some-component.vue, the module search path is relative to the vue2-jest source file and not relative to some-component.vue like you would expect. The end result is that it finds the top-levelsass-library@1.0.0and not the v3 version.To fix this, I am now passing in the optional "paths" value to require.resolve (available since node 8.9 https://nodejs.org/api/modules.html#requireresolverequest-options) and providing the path to the source file being processed instead. This will start the module resolution search at some-component.vue and the nested `node_modules/sass-library`` v3 will be found instead.