Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BREAKING CHANGE] remove inline source-map on process() output #255

Merged
merged 4 commits into from
Aug 8, 2020

Conversation

nogic1008
Copy link
Collaborator

@nogic1008 nogic1008 commented Jul 23, 2020

Resolve #241 and jestjs/jest#10089

In jest@25.5.0 or later, coverage reporter creates an inline source-map from map property. (jestjs/jest#9811)
But vue-jest already adds inline source-map with process().
So reporter throws error due to duplicate source maps.

This PR avoids duplicates by removing map property inline source map.

@lmiller1990
Copy link
Member

Cool, but won't this be a breaking change for anyone on an old version of jest?

We need a good strategy to handle this. I wonder if we can detect the version of jest somehow (maybe by importing package.json?)

Any ideas?

@nogic1008
Copy link
Collaborator Author

nogic1008 commented Jul 29, 2020

Cool, but won't this be a breaking change for anyone on an old version of jest?

We need a good strategy to handle this. I wonder if we can detect the version of jest somehow (maybe by importing package.json?)

Any ideas?

This package specifies jest@^25.x in peerDependencies.
The jest versions matching 25.x are:

  • 25.5.4, 25.5.3, 25.5.2, 25.5.1, 25.5.0
  • 25.4.0
  • 25.3.0
  • 25.2.7, 25.2.6, 25.2.4, 25.2.3, 25.2.2, 25.2.1, 25.2.0
  • 25.1.0
  • 25.0.0

>=25.2.3 cannot create SFC coverage due to jest's bug. (ref. #217)
So, I think we should check for breaking changes in the following versions.

  • 25.4.0
  • 25.3.0
  • 25.2.4-25.2.7

@nogic1008
Copy link
Collaborator Author

nogic1008 commented Jul 29, 2020

Memo:
This issue is caused @jest/transform>=25.5.0. (dependencies of jest)
Therefore, depending on the installation timing, it may occur even jest@25.4.0 or below. (because jest@25.4.0 specifies @jest/transform:^25.4.0)

@nogic1008
Copy link
Collaborator Author

@lmiller1990
jest@25 requires @jest/transform:^25 (including @jest/transform@25.5.0) for its dependencies.
So, its bug occurs even jest@25.4.0 or below. (ex. recreate package lock file)

This version is still in beta.
I think it's better to consider this PR as a breaking change and bump jest to ^25.5.0.

@nogic1008 nogic1008 changed the title fix: create single inline source-map on generate coverage [BREAKING CHANGE] remove inline source-map on process() output Aug 7, 2020
@lmiller1990
Copy link
Member

lmiller1990 commented Aug 8, 2020

Ok let's do it. Most people should be easily able to move to Jest 25. Can you write a short "migration guide" explaining the breaking change and what people might need to do to upgrade so we can include that in the release notes?

For now I will merge this, but if you can include the upgrade notes here (just as a comment) I can include that with the release.

I will do a release in the next few days.

@lmiller1990 lmiller1990 merged commit acc358f into vuejs:master Aug 8, 2020
@nogic1008
Copy link
Collaborator Author

@lmiller1990

Ok let's do it. Most people should be easily able to move to Jest 25. Can you write a short "migration guide" explaining the breaking change and what people might need to do to upgrade so we can include that in the release notes?

For now I will merge this, but if you can include the upgrade notes here (just as a comment) I can include that with the release.

I will do a release in the next few days.

What about this one?

Breaking Change

In this version, vue-jest no longer generates inline source maps. (because jest>=25.5.0 makes it too)
Source maps are used for coverage report.
So if you want to collect coverage, upgrade jest to >=25.5.0.

# npm
> npm install jest@^25.5.0
# yarn
> yarn add jest@^25.5.0

@nogic1008 nogic1008 deleted the hotfix/jest-transform branch August 17, 2020 01:50
@aantipov
Copy link

Hi there. Thanks for the fix!
Could you please also merge the changes into v4.0.0-beta? I had to turn off the coverage report before the fix arrives

Btw, I'm confused about having v4-beta and v5-alpha both at the same time.
v4-beta has been there for a while and still not achieved a stable label.

Is there any information about the future of both versions?

@lmiller1990
Copy link
Member

This was merged into 4.0.0-beta. Maybe we need to do a release? I am not sure if we did once since this was merged. Can you check?

Basically:

v4 = Vue 2
v5 = Vue 3

I can make the next release rc.1 for v4... the release goal isn't super clear, but seems many people are using it fine in production.

@aantipov
Copy link

aantipov commented Sep 6, 2020

Maybe we need to do a release? I am not sure if we did once since this was merged. Can you check?

I see that the latest 4-beta was released on 2 Aug which happened before the fixes.
Also this page shows that the hotfix is not in the latest 4.0.0-beta.5.
So, yeah, we do need a new release of the 4-beta, or even make it rc

image

@matt-boyd-msm
Copy link

Is it possible to get this fix in v5 too please?

@lmiller1990
Copy link
Member

I released a new 4.0.0 today. So it should be fixed there. This change

Is someone able to try porting this to 5.0.0? It should be quite easy. I have many other things to do in the meantime for Vue 3 release. Otherwise I can do it next week. If someone can do it sooner I can do a release just for this.

Btw, I am exploring some ideas for a ts-jest based vue-jest here: https://github.com/lmiller1990/vue-ts-jest. If that goes well maybe we can use that instead of 5.0.0 for Vue 3 (I think TS in tests is very useful; we don't get diagnostics using the babel based workflow in this codebase).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jest coverage reports cause an error with v4.0.0-beta.2
5 participants