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.

Sign up for GitHub

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

Jump to bottom

Not compatible with [email protected] #195

Closed
FrozenPandaz opened this issue Sep 19, 2018 · 5 comments · Fixed by #204
Closed

Not compatible with [email protected] #195

FrozenPandaz opened this issue Sep 19, 2018 · 5 comments · Fixed by #204

Comments

@FrozenPandaz
Copy link
Contributor

Current Behavior

[email protected] (published today) is a rewrite of the package and has many breaking changes

yarn list v1.9.4
warning Filtering by arguments is deprecated. Please use the pattern option instead.
├─ [email protected]
│  └─ [email protected]

Expected Behavior

Short term, lock ts-jest to ~23.1.0.

Long term, ensure compatibility with ts-jest@^23.10.0.
@ahnpnl
Copy link
Collaborator

ahnpnl commented Sep 19, 2018

PR is welcome 🙂

@FrozenPandaz FrozenPandaz mentioned this issue Sep 19, 2018
Merged
thymikee pushed a commit that referenced this issue Sep 19, 2018
@FrozenPandaz @thymikee
fix: restrict version of ts-jest to ~21.1.3 (#196)
5701446 
fixes short term #195
thymikee pushed a commit that referenced this issue Sep 19, 2018
@FrozenPandaz @thymikee
fix: restrict version of ts-jest to ~23.1.3 (#196)
62673cf 
fixes short term #195
@ahnpnl ahnpnl mentioned this issue Sep 21, 2018
Closed
@FrozenPandaz FrozenPandaz mentioned this issue Sep 28, 2018
Closed
@byronmejia
Copy link

Thanks for the fix @FrozenPandaz ! Such a shame that kulshekhar/ts-jest can't follow semver...

How can we help with ensuring compatibility with 23.1.0? Any places to start at, or any tips?

@ahnpnl
Copy link
Collaborator

ahnpnl commented Oct 3, 2018

Hi @byronmejia

I contacted with ts-jest's contributors and got some clues to start but I am on vacation now so I don't have much time to look into it. You can contact @huafu to discuss a point to start :)

@byronmejia
Copy link

Awesome! Cheers. :)

@ahnpnl ahnpnl mentioned this issue Oct 4, 2018
Closed
@danil-z danil-z mentioned this issue Oct 17, 2018
Closed
@huafu huafu mentioned this issue Oct 17, 2018
Closed
@huafu
Copy link

huafu commented Oct 17, 2018

@danil-z and others, here is my WIP PR #201, since I have no time for now to continue working on it. The big stuff remaining is explained in the header of the preprocessor.js file which itself needs to be removed.

@wtho wtho mentioned this issue Nov 28, 2018
Merged
thymikee pushed a commit that referenced this issue Dec 4, 2018
@wtho @thymikee
feat(breaking): Implement astTransformer compatible with ts-jest 23.10 (
adad842 
#204)

Closes #195 and closes #201

### Breaking Change
This is a breaking change as Configuration Interface changes ([see ts-jest v23.10 docs](https://kulshekhar.github.io/ts-jest/user/config/#ts-jest-options))

This PR is built upon @huafu's #201, kudos to him for setting up everything and pointing out what was still to be done. Transformer and test are also heavily inspired by his examples in ts-jest. Also see the PR for more information.

### Implementation Details
Like the Regex, all the transformer does is transform
* `templateUrl: <PATH>` to `template: require(<PATH>)`
* `styles: <ANYTHING>` to `styles: []` 
* `styleUrls <ANYTHING>` to `styleUrls: []`

For more information see the comments in the [transformer file](https://github.com/wtho/jest-preset-angular/blob/55bcfdb993d1dc511611553995ff566d64ea6944/src/InlineHtmlStripStylesTransformer.ts) or #201.

### Edge Cases
I had to make some decisions on how strict the astTransformer is, I decided for this, but I am open to discussion:
**Works**
```ts
import { Component as Cmp } from '@angular/core';
@cmp({
  templateUrl: './some-path.html'
})
class AngularComp { }
```
Caveat - this also gets transformed
```ts
@SomeOtherDecorator({
  templateUrl: './some-path.html'
})
class SomeClass { }
```

**Does not work**
```ts
const componentArgs = {
  templateUrl: './some-path.html'
}
@component(componentArgs)
class AngularComp { }
```

### Additional Notes
* Run unit tests for the transformer with `npm test`
* The transformer was written in TypeScript. A `prepare`-script for was added, which compiles it to JS before publishing and after `npm install`.
* `preprocessor.js` and its unit tests were removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement astTransformer - Compatibility with ts-jest 23.10
4 participants
@huafu @FrozenPandaz @byronmejia @ahnpnl