vuejs / vue-jest

Jest Vue transformer
MIT License
746 stars 157 forks source link

fix: allow importing relative paths in global resources #373

Closed pmrotule closed 2 years ago

pmrotule commented 3 years ago

Closes #374

Fix importing relative paths inside a stylesheet being made globally available using the option jest.globals['vue-jest'].resources:

{
  "jest": {
    "globals": {
      "vue-jest": {
        "resources": {
          "scss": [
            "./src/styles/settings/global.scss"
          ]
        }
      }
    }
  }
}

src/styles/settings/global.scss

@import './unit.scss';

$another-variable: 10px;

considering that src/styles/settings/unit.scss exists, it shouldn't throw the following error

image

Version

My branch is based on the v4.0.1 tag which is the version I'm using. It doesn't seem like there is a v4 branch I could base my PR on.

pmrotule commented 3 years ago

Are you able to resolve the conflicts?

@lmiller1990 I can't unfortunately. It is actually on the latest v4 code, but there is no v4 branch that I could based this PR on. It is currently based on master, but the master branch is now a monorepo for v26. I think making a v4 branch would make sense and get rid of the conflicts. I would need your help to create the branch though.

FYI: yarn test:e2e style was failing with Less and I just fixed it with my latest commit. Should be good to go now.

lmiller1990 commented 3 years ago

Sorry about the delay on this.

If we can resolve the conflicts, we can merge!

pmrotule commented 3 years ago

If we can resolve the conflicts, we can merge!

:warning: There is no conflict.

@lmiller1990 It seems like you missed my explanation about the source of the conflicts. In https://github.com/vuejs/vue-jest/issues/374#issuecomment-913257195, you confirmed that v4 was the correct version and this PR is currently on the latest v4 code. We only see conflicts because the PR is currently based on the master branch which is v27:

Screenshot 2021-09-27 at 14 45 15

but there is no v4 branch, only a v3 one.

TLDR;

To fix the conflicts, you need to create a v4 branch for me to base the PR on. Or should I fix it in v3?

lmiller1990 commented 3 years ago

Right, sorry, I did not explain the context.

In order to support both Vue 2 and Vue 3 (and Jest 26.x and 27.x) we moved to a monorepo.

There are two packages: vue2-jest and vue3-jest. Since vue-jest is built for Jest, we need to follow their versioning scheme to ensure compatibility.

We have two branches:

image

The code for v4, which works with Vue 2, is in vue2-jest now. If you are on Jest 26, you'd submit your patch against vue2-jest on the v26 branch. For Jest 27, it'd be on the vue2-jest package on master.

Ideally we want to be patching the latest version of Jest, which is v27, but if you are still on v26 you could submit the patch against that branch/package and we could release it.

Sorry if that's confusing, if it's still not clear, let me know.

lmiller1990 commented 2 years ago

Hi! Can you resolve the conflicts? Then we can get this merged up. Let me know if my above comment is confusing.

pmrotule commented 2 years ago

@lmiller1990 I'm sorry, I'm currently on vacation, but I'm back on Monday and can resolve the conflicts on the v26 branch. Thanks for your explanation, it is much clearer now and sorry for the back and forth. I can probably also fix it in v27, we just didn't use it in our project yet since it is still in alpha version. We're looking forward to use Jest v27 though and Nuxt 3 is around the corner so we will soon finally switch to Vue 3.

I'll keep you posted.

pmrotule commented 2 years ago

@lmiller1990 I was able to resolve the conflicts by rebasing my branch on 26.x, but I'm still confused about the next steps. I tested my changes locally with my own project, but I didn't know how to run the e2e tests until I realized there was no test running on the CI. I guess that's why it is still an alpha version, but considering this, shouldn't I be fixing the bug on the stable version?

I also noticed something wrong in the e2e package.json files: it refers to vue2-jest instead of @vue/vue2-jest or vue-jest. It seems to be fixed on the master branch, but it still doesn't make sense to refer to a specific version on npm instead of the local version.

I decided to go ahead and fix the e2e test run in https://github.com/vuejs/vue-jest/pull/373/commits/4222d2364824cb887ecf5a07bbf429082f541202 using the local versions like I suggested. It's out of scope so let me know if you would rather have it in a separate PR. If you like it, I'm happy to submit a new PR to implement it for v27.

pmrotule commented 2 years ago

@lmiller1990 Any chance we can merge this PR?

pmrotule commented 2 years ago

@lmiller1990 Please?

lmiller1990 commented 2 years ago

@pmrotule Yes, sorry.

lmiller1990 commented 2 years ago

@pmrotule pls try it out: https://github.com/vuejs/vue-jest/releases/tag/v26.0.1

We now use 26.x for Jest 26 support. You changes should be on @vue/vue2-jest@26.0.1.

Let me know if you have any problems.

pmrotule commented 2 years ago

@lmiller1990 Thanks! I just tested @vue/vue2-jest@26.0.1 in my project and it works like a charm 🎉

We are currently working on upgrading to Vue 3 so I should be able to create a PR with the same fix for Vue 3 in the next few days.

lmiller1990 commented 2 years ago

Great, thanks. I'll try to be more responsive with merging and releasing new versions.