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

Instructions do not work on Angular 10 CLI project #1

Closed
IanIsFluent opened this issue Sep 21, 2020 · 25 comments
Closed

Instructions do not work on Angular 10 CLI project #1

IanIsFluent opened this issue Sep 21, 2020 · 25 comments
Labels
help wanted Extra attention is needed

Comments

@IanIsFluent
Copy link

I have an Angular 10 CLI project (v10.1.2). I've installed @testing-library/angular@10.0.2, but as I'm using Karma (configured via Angular) and Typescript, I'm not sure how to install this library!

I tried following the instructions for the latest version (v1.1.0), but while I can see the matchers on the HTMLMatchers type in VSCode, I can't actually add the Matchers, because trying to import them with import JasmineDOM from '@testing-library/jasmine-dom'; gives an error File C:/.../node_modules/@types/testing-library__jasmine-dom/index.d.ts is not a module.

As this is an Angular CLI project, would it actually make more sense to use a karma plugin!?

@IanIsFluent
Copy link
Author

I also tried the previous version. And while this time I could import JasmineDOM from '@testing-library/jasmine-dom';, I couldn't get my Typescript test to compile, because it didn't know about the extra matchers on the HTMLMatchers type 😦

@brrianalexis
Copy link
Member

Hi! Thanks for checking out the library!

I'm sorry but unfortunately I'm not familiarized at all with how Karma works. So far I've only been using the library to test a React app at work.

From what I've been reading today, a plugin is needed to add the matchers. Actually, the most popular library for jasmine matchers has a karma plugin to handle this.

I'd try to have a look at it but I don't really have the time to study Karma and develop the plugin at the moment. You're more than welcome to do so if you can and want to!

If you come up with a solution or develop the plugin, a PR to update the docs would be great 😄

@brrianalexis brrianalexis added the help wanted Extra attention is needed label Sep 21, 2020
@IanIsFluent
Copy link
Author

IanIsFluent commented Sep 27, 2020

Hi, I've taken a look at how the library you linked works - and I think all its doing is adding a file to the files array in karma.conf.js.

Like:

    files: [
      require
        .resolve("jasmine-expect")
        .replace("index.js", "dist/jasmine-matchers.js"),
    ],

So I thought we just need to do that for your library - but in trying to do that I get Uncaught ReferenceError: exports is not defined :(

Error during loading: Uncaught ReferenceError: exports is not defined in http://localhost:9877/basesrc/...../angularproj/node_modules/@testing-library/jasmine-dom/dist/index.js?5cb45e526f75ddf43f0416f7ff4f3a9d3c8b6bf0 line 3

It seems that jasmine-matchers.js source starts with some webpackBootstrap comments - whereas yours doesn't... According to this: https://stackoverflow.com/questions/33294705/gulp-babel-exports-is-not-defined it'll be because babel doesn't give you something you can run in the browser - webpack / browserify do that... So I'd need to add webpack to the @testing-library/jasmine-dom repo.

To test I just:

  • Installed angular command line globally
  • Ran ng new to create a new project
  • npm i --save-dev '@testing-library/jasmine-dom
  • added to karma.conf.js: files: [require.resolve('@testing-library/jasmine-dom']

Then saw the error 😦

@IanIsFluent
Copy link
Author

IanIsFluent commented Sep 27, 2020

In order to try to get around the error importing JasmineDOM:
'{...}node_modules/@types/testing-library__jasmine-dom/index.d.ts' is not a module..

Because of that error I tried just adding @testing-library/jasmine-dom to types in tsconfig.spec.ts - but that gives me a big error from the test:

ERROR in ./node_modules/@testing-library/jasmine-dom/node_modules/css/lib/stringify/source-map-support.js
Module not found: Error: Can't resolve 'fs' in 'C:\src\npmlink-try\angularproj\node_modules\@testing-library\jasmine-dom\node_modules\css\lib\stringify'
resolve 'fs' in 'C:\src\npmlink-try\angularproj\node_modules\@testing-library\jasmine-dom\node_modules\css\lib\stringify'

@IanGrainger
Copy link
Contributor

Just looked some more - and the css module is the issue. If I remove the toHaveStyle matcher - I can get all your code to work in a new Angular project! (I also had to remove an optional chain on line 13 of toBeChecked because its a js file - and I guess Angular or node didn't like that (it works fine in ts files).

@IanGrainger
Copy link
Contributor

IanGrainger commented Oct 18, 2020

I've forked you here: https://github.com/IanGrainger/jasmine-dom and removed the toBeChecked matcher, so I can use the project. I'll try to work out how to change it to a re-export of your code, rather than a fork sometime! 👍

I've also published my fork to npm here: https://www.npmjs.com/package/@iangrainger/jasmine-dom - hope that's OK!

Thanks!

@brrianalexis
Copy link
Member

Hey, that's great!

I believe ideally we should try to update this module, but I think that adding your fork to the docs will be fine as a temporary solution.

If you want to update the docs to reference your fork, that would be great. Otherwise, I'll do it over the weekend!

I'll leave this issue open for further discussion until we come up with a solution.

Thank you for your work!

@IanGrainger
Copy link
Contributor

Yes - of course it'd be better to fix this repo! But I'm not sure what to do about the broken css library. Apparently there's a workaround: webpack-contrib/css-loader#447 (comment) - but I couldn't get it to work! :(

@IanGrainger
Copy link
Contributor

IanGrainger commented Oct 26, 2020

I now have 2 open PRs on the CSS library to get a workaround to allow it to work for parsing in the browser. It will still fail to stringify with sourcemap support - but that's not something you're using here.

I started making a version of this repo in typescript for the other issue, because Angular uses it, you need to add to the global namespace to get the tests to compile - but by the time I got to setting up webpack to get the tests to run I thought it was getting to a lot of effort! 😉 I do now have an example of the changes I'd need to add to the d.ts though...

declare global {
    namespace jasmine {
        interface Matchers<T> {
            toHaveAttribute(attr?: string, value?: string): void;
            toHaveTextContent(): void;
            toHaveClassName(...attrs: (string | {
                exact: boolean;
            })[]): void;
            toBeChecked(): void;
            toBeEmptyDOMElement(): void;
            toHaveFocus(): void;
            toBeDisabled(): void;
            toBeEnabled(): void;
            toHaveDescription(text?: string | RegExp): void;
            toHaveValue(): void;
            toHaveFormValues(): void;
            toContainElement(el?: Element): void;
            toBeRequired(): void;
            toBeInvalid(): void;
            toBeValid(): void;
            toHaveDisplayValue(text?: string | RegExp | (string | RegExp)[] | boolean): void;
            toBePartiallyChecked(): void;
            toBeInTheDocument(): void;
            toBeVisible(): void;
        }
    }
}
  • some of them have typings and some don't - that was just getting the tests to compile.

@IanGrainger
Copy link
Contributor

@brrianalexis still have two PRs waiting approval on the CSS library :( any idea what I have to do to get someone to approve them? I'm new to contributing to npm (if that wasn't obvious)... Pay them? 😆

I'm wondering if an alternative would be a new library that just exports the bit of the CSS library you need here? a css-parse ibrary?

@brrianalexis
Copy link
Member

@IanGrainger hey! I'm really sorry for dropping the ball on this one. I've been really busy lately focused on getting a new job.

I'm new to npm and open source in general too, don't worry 😅

Let's see if I understood correctly:

I do now have an example of the changes I'd need to add to the d.ts though...

By this you mean the declaration file, right? If that's the case, then you can change it here in types/testing-library_jasmine-dom. If you do make a PR to add those changes, then let me know and I'll review it so we can get it merged as asap as possible 😄.

I'm wondering if an alternative would be a new library that just exports the bit of the CSS library you need here? a css-parse ibrary?

That would definitely be an alternative. I've thought about it myself too. I don't know if it would be too much work to do it 🤔

@IanIsFluent
Copy link
Author

@brrianalexis no worries, I hope you got a good job! 👍

I've pr'd the global namespace change here: DefinitelyTyped/DefinitelyTyped#49510.

Re: reexporting, I wasn't sure if the parse bit that you use is separable enough or not... I'd have to go back and look in their library.

@IanIsFluent
Copy link
Author

Oh dear, looks like there's some problems with the global namespace stuff - mainly that it isn't allowed, apparently... https://github.com/DefinitelyTyped/DefinitelyTyped/runs/1390683036

@brrianalexis
Copy link
Member

I'm not a typescript expert at all, but have you tried with just changing the already existing definitions? 🤔

@IanIsFluent
Copy link
Author

IanIsFluent commented Nov 13, 2020 via email

IanGrainger added a commit to IanGrainger/jasmine-dom that referenced this issue Dec 9, 2020
@IanGrainger
Copy link
Contributor

reworkcss/css#146 have suggested a better fix. Just import the parse function - PR here: #8

@IanIsFluent
Copy link
Author

IanIsFluent commented Dec 9, 2020

To allow people to use Karma, I think you still need to add a line to your Getting Started: files: add to karma.conf.js: files: [require.resolve('@testing-library/jasmine-dom']

IanGrainger added a commit to IanGrainger/jasmine-dom that referenced this issue Dec 10, 2020
@brrianalexis
Copy link
Member

brrianalexis commented Dec 13, 2020

Now that #8 is merged, do we need to update the docs? If so, you're more than welcome to do it. Otherwise, let me know and I'll update them!

@IanIsFluent
Copy link
Author

IanIsFluent commented Dec 14, 2020

Docs need:

Usage with Karma and Typescript in an Angular 10 project:

  1. Add to the existing types array in tsconfig.spec.json: "@testing-library/jasmine-dom" (mine is "types": ["jasmine", "@testing-library/jasmine-dom"])
  2. Pass dist/index to jasmine matchers in test setup file test.ts:
import JasmineDOM from '@testing-library/jasmine-dom/dist';
...
beforeAll(() => jasmine.addMatchers(JasmineDOM));

I think adding to types means we don't need to add to the global namespace as I suggested above.

Does it definitely work with Jest when you import from '@testing-library/jasmine-dom' rather than '@testing-library/jasmine-dom/dist'?

@IanIsFluent
Copy link
Author

@brrianalexis I wonder if you could add a file which does: import "x" / beforeAll((()=> jasmine.addMatchers(x)) into the dist of this library? then we could add that file to the tsconfig.spec.json / jasmine conf files array? Maybe?

@brrianalexis
Copy link
Member

Does it definitely work with Jest when you import from '@testing-library/jasmine-dom' rather than '@testing-library/jasmine-dom/dist'?

This package is for Jasmine. If you're using Jest, it's a bit more simple to set up using jest-dom

In my case, I always imported directly from @testing-library/jasmine-dom but I always worked with JavaScript, not TypeScript so I'm not sure about importing from /dist. If you need to import from /dist using TypeScript, then we should add that to the docs too.

@brrianalexis I wonder if you could add a file which does: import "x" / beforeAll((()=> jasmine.addMatchers(x)) into the dist of this library? then we could add that file to the tsconfig.spec.json / jasmine conf files array? Maybe?

I'm not really sure about that. Have you tried it in your fork?

@IanIsFluent
Copy link
Author

IanIsFluent commented Dec 14, 2020

Does it definitely work with Jest when you import from '@testing-library/jasmine-dom' rather than '@testing-library/jasmine-dom/dist'?

This package is for Jasmine. If you're using Jest, it's a bit more simple to set up using jest-dom

Sorry, I meant whatever you were using, so Javascript, not Jest 😆.

@brrianalexis I wonder if you could add a file which does: import "x" / beforeAll((()=> jasmine.addMatchers(x)) into the dist of this library? then we could add that file to the tsconfig.spec.json / jasmine conf files array? Maybe?

I'm not really sure about that. Have you tried it in your fork?

It was just so people didn't need to add the file themselves - seeing as the content is always the same... But it differs if you're using JS or TS it seems!

@brrianalexis
Copy link
Member

Usage with Karma and Typescript in an Angular 10 project:

  1. Add to the existing types array in tsconfig.spec.json: "@testing-library/jasmine-dom" (mine is "types": ["jasmine", "@testing-library/jasmine-dom"])
  2. Pass dist/index to jasmine matchers in test setup file test.ts:
import JasmineDOM from '@testing-library/jasmine-dom/dist';
...
beforeAll(() => jasmine.addMatchers(JasmineDOM));

Awesome. Is this all that needs to be added to the docs?

@IanIsFluent
Copy link
Author

Usage with Karma and Typescript in an Angular 10 project:

  1. Add to the existing types array in tsconfig.spec.json: "@testing-library/jasmine-dom" (mine is "types": ["jasmine", "@testing-library/jasmine-dom"])
  2. Pass dist/index to jasmine matchers in test setup file test.ts:
import JasmineDOM from '@testing-library/jasmine-dom/dist';
...
beforeAll(() => jasmine.addMatchers(JasmineDOM));

Awesome. Is this all that needs to be added to the docs?

Yep, that'll work, great 👍 (or at least it did in my test project).

Only thing I was toying with was suggesting you add a new file to the files array in karma.conf.ts and just add the beforeAll there - but lets stick with this 😄

@brrianalexis
Copy link
Member

@IanGrainger, I've put up a new PR #10 to update the docs. Please review it and feel free to suggest any changes 🙏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants