thebuilder Github contribution chart
thebuilder Github Stats
thebuilder Most Used Languages

Activity

26 Sep 2022

Thebuilder

v1.3.0

Pushed On 26 Sep 2022 at 07:47:40

Thebuilder

upgrade dependencies

Pushed On 26 Sep 2022 at 07:45:49
Issue Comment

Thebuilder

[FEATURE] Mocks for testing

Is your feature request related to a problem? Please describe. Testing code that uses framer-motion in a jsdom environment is considerably slowed down. So a mock that would bypass the animation steps would help a lot. Unfortunately, the API is rather involved and thus hard to keep in sync; it would be simpler for the package itself to provide mocks.

Describe the solution you'd like A 'framer-motion/mocks' export that can be used in testing would be the best solution IMO.

Describe alternatives you've considered One can still mock only a few APIs, but that is brittle and requires extra work.

Additional context Testing was done with jest and vitest. Both allow for filename alliasing.

Forked On 25 Sep 2022 at 04:41:23

Thebuilder

This would great. Being able to skip LazyMotion and animations in general would help a lot with making tests faster and more reliable. I'm running in to a bunch of act() warnings when upgrading to the latest version of @testing-library/dom, and it seems to be related to LazyMotion components.

Commented On 25 Sep 2022 at 04:41:23

Thebuilder

threshold does not work in Chrome / webkit

Edge / Chrome both ignore threshold--essentially everything is treated as a value of 0. Or in other words, the event triggers as soon as the first pixel is visible. Firefox on the other hand has correct behavior and you can see differences between values from 0 to 1. I have not tried Safari yet.

Forked On 14 Sep 2022 at 08:26:51

Thebuilder

threshold is working fine in Chrome/Edge - You can test it on the demo site: https://react-intersection-observer.vercel.app/?path=/story/useinview-hook--with-threshold-50-percentage If you are seeing issues, it's most likely related to how you are using the IntersectionObserver. Maybe styling is not the same between browsers - e.g. height is collapsed on the element you are observing?

If you believe it to still be an issue, please provide a reproducible example.

Commented On 14 Sep 2022 at 08:26:51

Thebuilder

Many targets with one observer.

Can I use "react-intersection-observer" to observe for many elements(targets) in one root component and one observer?

Forked On 11 Sep 2022 at 05:50:46

Thebuilder

It happens automatically if the observer is using the same options. Just create your useInView hooks where you need to them.

Commented On 11 Sep 2022 at 05:50:46

Thebuilder

Rootmargin

Root margin with chat isn't working when scroll up. Please check my situation

Forked On 07 Sep 2022 at 01:08:20

Thebuilder

This issue tells me nothing.

Maybe check the FAQ? https://github.com/thebuilder/react-intersection-observer#rootmargin-isnt-working-as-expected

Commented On 07 Sep 2022 at 01:08:20

Thebuilder

test fixes

Pushed On 31 Aug 2022 at 12:42:44

Thebuilder

Merge pull request #1 from malt0533/test-fix

Pushed On 31 Aug 2022 at 12:42:44

Thebuilder

test fixes

Created On 31 Aug 2022 at 12:42:43

Thebuilder

Some dependencies don't support ES modules

Edit 2: Per request, I've simplified this down to a few simple questions and answers.

What did you do? I attempted to use dom-testing-library in a test suite that's be run via Web Test Runner.

What happened? The test runner bombed out complaining about CJS modules that are dependencies of dom-testing-library: aria-query, corejs, and lz-string. There may be more but that's as far as I spent time digging into them. image

What did you expect to happen? I expected the test runner to actually run without dependency errors.

Is there a repro? https://github.com/jimsimon/wtr-dtl/tree/wtr-using-wds (note the branch name)

What have you tried so far? I've tried using the Web Test Runner's CJS support that leverages Rollup's CJS plugin but that did not help. It appears the dependencies causing the errors are not supported by Rollup's CJS plugin.

Is there a proposed solution? Upgrade to versions of these dependencies that support native ES modules or migrate to alternatives that do.

Any other notes? One of the conflicting dependencies (aria-query) was updated in #1058 so that one may no longer be problematic. I haven't had a chance to test it though. I just tried the repro with the latest versions of all dependencies and I'm still having issues.

Edit: This issue originally started as a request to have dom-testing-library broken down into separate packages to make it easier to consume by non-Jest based test runners. It has since transformed into a discussion about what's preventing dom-testing-libary form working with web-test-runner. In an effort to focus this ticket, I've copied the relevant parts of the conversation below.

First a TLDR: dom-testing-library does not work with web-test-runner. Multiple dependencies used by dom-testing-library do not publish native esm modules and are not compatible with rollup's commonjs plugin (which can be used by web-test-runner). One big offender is aria-query, which may be fixed with #1058 but has not been tested yet. Main repro can be found in https://github.com/jimsimon/wtr-dtl/tree/wtr-using-wds (note the branch name since there are multiple different repros for different attempts to workaround the problem in this repo).

The main problem is that web-test-runner is designed for "buildless" environments using entirely native ESM in the browser. Dom-testing-library has a handful of dependencies that only provide CJS modules that don't play nice in this environment. You can actually use the rollup commonjs plugin for some of these, but some still don't work. Specifically, I recall running into issues with aria-query, corejs, and lz-string. The corejs dependency is some sort of weird prebundled one coming transitively via aria-query.

I'll try to create a sample repo tomorrow and provide a link here, but it's pretty simple to replicate. Create a brand new node project, add web-test-runner and dom-testing-library, write a simple test that uses screen, run the tests via web-test-runner. You'll see web-test-runner just choke and/or throw an error about the CJS deps.

The most success I've had was using snowpack with their web-test-runner plugin in "remote" dependency mode. Unfortunately even that approach errors out.

My suggestion to split the queries out to a separate package was to hopefully reduce the dependency load. A better approach might be to work towards updating to dependencies that also provide native ESM distributions (or remove them altogether).

-- @jimsimon

Does web-test-runner use the module field in package.json?

-- @nickmccurdy

It does as it actually uses https://github.com/rollup/plugins/tree/master/packages/node-resolve under the hood for them. Just for clarity sake, web-test-runner is built on top of web-dev-server, which is what actually handles file transformation.

The main issue seems to be some dependencies that aren't providing ESM distributions. Aria-query in specific is keeping corejs3-runtime as a dependency to support people still on Node 6. There's a ton of discussions about that in https://github.com/A11yance/aria-query/issues/158 and a PR to clean a lot of it up here: https://github.com/A11yance/aria-query/pull/180. So it seems part of the solution here may be to encourage that PR to move forward and then update dom-testing-library accordingly? There are still other dependencies that may need updating as well, but maybe there's some alternatives that provide ESM output available?

-- @jimsimon

I pushed up a couple versions of my demo repo:

Snowpack using remote sources (main branch): https://github.com/jimsimon/wtr-dtl Snowpack using local sources (snowpack-local branch): https://github.com/jimsimon/wtr-dtl/tree/snowpack-local WTR using it's built in Web Dev Server + ESBuild (wtr-using-wds branch): https://github.com/jimsimon/wtr-dtl/tree/wtr-using-wds Hopefully these help. The last link is the one I'd most like to see working and is what spawned this ticket and convo. The Snowpack stuff was an attempt at working around the issue.

Note: The test won't actually pass if you get it to run. That's a different issue/concern regarding being able to pass ShadowRoot to queries. I'm hoping we can address that in a different issue/ticket if we can get this one resolved.

-- @jimsimon

Note that @testing-library/dom itself would not work with native ES modules. But then I see mentions of aria-query which may be fixed with https://github.com/testing-library/dom-testing-library/pull/1058.

So could somebody summarize what the issue is? The issue description is currently missing what is concretely not working (ideally with a repro).

-- @eps1lon

This is the end of the conversation as of this edit. Hopefully this helps!

Vote on feature requests by adding a 👍. This helps maintainers prioritize what to work on.

  • Please fill out this template with all the relevant information so we can understand what's going on and fix the issue. We appreciate bugs filed and PRs submitted!

  • Please make sure that you are familiar with and follow the Code of Conduct for this project (found in the CODE_OF_CONDUCT.md file).

It'd be great if after the discussion you're the one who submits the PR that implements this feature. If you've never done that before, that's great! Check this free short video tutorial to learn how: https://kcd.im/pull-request

If this is an issue with the documentation, please file an issue in the docs repo: https://github.com/testing-library/testing-library-docs -->

Describe the feature you'd like:

I'd love to be able to use the queries provided by testing-library in other testing frameworks. This would allow for a gradual migration to testing-library for codebases with existing test frameworks/runners that aren't compatible with testing-library. One example of a test runner that doesn't play nicely with dom-testing-libary is web-test-runner. Splitting the queries out to a separate package would allow more people to use them and help improve testing in the larger JS ecosystem. This should be doable without any breaking changes, as the existing API's could just delegate to the new package.

Suggested implementation:

Create a new @testing-library/queries package and move all query related code to it.

Describe alternatives you've considered:

The only alternative I can think of would be to reimplement the same queries in an entirely new project. This seems like wasted effort if we can simply extract the ones that testing-library already provides.

Teachability, Documentation, Adoption, Migration Strategy:

There should be no outward facing impact on users as API's would remain backwards compatible. Documentation for the new package would need to be created, and some minor documentation updates might be needed for the main site.

Forked On 18 Aug 2022 at 08:06:08

Thebuilder

Running React with vitest, and after upgrading to 8.16.1 a bunch of my tests now fail with act() warnings. Not sure of all the triggers, but I've narrowed one issue down to having a useQuery hook that's called, so it performs an async data fetch.

Commented On 18 Aug 2022 at 08:06:08

Thebuilder

started

Started On 11 Aug 2022 at 09:10:10

Thebuilder

infinite loop caused by `isInViewRef`

When calling setRef from this package, it causes an infinite loop if called from within an inline function e.g. when passed as a param to packages like react-merge-ref.

Here's a simple repro of the issue (calling manually from inside an inline ref callback):

https://codesandbox.io/s/sweet-rgb-6werdb?file=/src/App.js

When an inline ref function is used, React will set the ref to null and back again on every render. So, from my quick dive into this package's code I believe this caveat is causing the observer to set up again which is firing setState and re-rendering, which calls setRef with null (and back again), and loops.

Forked On 08 Aug 2022 at 05:54:07

Thebuilder

It's doing 3 renders at the moment. I tried different solutions, but the others still had issues. So ended up with useState

Commented On 08 Aug 2022 at 05:54:07

Thebuilder

Update Dockerfile

Pushed On 02 Aug 2022 at 09:50:36

Thebuilder

v1.2.0

Pushed On 01 Aug 2022 at 07:35:33

Thebuilder

v1.1.2

Pushed On 01 Aug 2022 at 07:35:11

Thebuilder

chore: upgrade dependencies

Pushed On 01 Aug 2022 at 07:35:11
Issue Comment

Thebuilder

[0.20.2]: Tets fails if importing `tslib`

Describe the bug

After upgrading to v0.20.x, most of my tests are now failing with:

SyntaxError: Named export '__assign' not found. The requested module 'tslib' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'tslib';
const { __assign } = pkg; 

It's caused by framer-motion using tslib. The package does include an exports field and module support, so a bit weird why this is popping up. Thinking it's related to how node modules are resolved now after #1673.

Reproduction

The issue can be replicated by importing <framer-motion> in a test file. https://stackblitz.com/edit/vitest-dev-vitest-ue9bu2?file=test/App.test.tsx

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (4) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 16.14.2 - /usr/local/bin/node
    Yarn: 1.22.10 - /bin/yarn
    npm: 7.17.0 - /bin/npm
  npmPackages:
    @vitejs/plugin-react: ^1.3.2 => 1.3.2 
    vite: ^2.9.9 => 2.9.14 
    vitest: 0.20.2 => 0.20.2 

Used Package Manager

npm

Validations

Forked On 31 Jul 2022 at 02:30:24

Thebuilder

Setting deps.registerNodeLoader to false (to skip the new Node logic) allows the test to run again.

Commented On 31 Jul 2022 at 02:30:24
Issue Comment

Thebuilder

[0.20.2]: Tets fails if importing `tslib`

Describe the bug

After upgrading to v0.20.x, most of my tests are now failing with:

SyntaxError: Named export '__assign' not found. The requested module 'tslib' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'tslib';
const { __assign } = pkg; 

It's caused by framer-motion using tslib. The package does include an exports field and module support, so a bit weird why this is popping up. Thinking it's related to how node modules are resolved now after #1673.

Reproduction

The issue can be replicated by importing <framer-motion> in a test file. https://stackblitz.com/edit/vitest-dev-vitest-ue9bu2?file=test/App.test.tsx

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (4) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 16.14.2 - /usr/local/bin/node
    Yarn: 1.22.10 - /bin/yarn
    npm: 7.17.0 - /bin/npm
  npmPackages:
    @vitejs/plugin-react: ^1.3.2 => 1.3.2 
    vite: ^2.9.9 => 2.9.14 
    vitest: 0.20.2 => 0.20.2 

Used Package Manager

npm

Validations

Forked On 31 Jul 2022 at 02:26:19

Thebuilder

Looks like tslib is not following the ESM standard correctly, and guessing it's the same issue with other packages. 😑 https://github.com/microsoft/tslib/issues/173

Commented On 31 Jul 2022 at 02:26:19
Issue Comment

Thebuilder

fix: apply Vite resolving algorithm to node_modules libraries

This PR adds experimental loader and conditions, so native Node resolving algorithm knows about Vite's resolving algorithm.

This is a giant breaking change, but still should not affect a lot of people. This fixes issues when people applied custom conditions and they were working in Vite, but didn't work in Vitest, because it was imported from another library. This PR also allows applying alias to external libraries - this is useful for preact, for example.

We still don't allow mocking external libraries within another external library!

Closes #1638

Forked On 31 Jul 2022 at 10:11:16

Thebuilder

After upgrading to v0.20.1, most of my tests are now failing with:

SyntaxError: Named export '__assign' not found. The requested module 'tslib' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'tslib';
const { __assign } = pkg; 

It seems to be caused by framer-motion using tslib. The package does include an exports field and module support, so a bit weird why this is popping up. Thinking it's related to how node modules are resolved now?

Commented On 31 Jul 2022 at 10:11:16

Thebuilder

Created with CodeSandbox

Forked On 27 Jul 2022 at 08:49:04

Thebuilder

`InView` component types are not compatible with the `@emotion/react`

Describe the bug

@emotion/react library allows to pass the css property into components if the className prop is a part of them. The InView has the className prop but typescript complains when passing the css prop.

@emotion/react types inject this property using the following type:

type WithConditionalCSSProp<P> = 'className' extends keyof P
  ? string extends P['className' & keyof P]
    ? { css?: Interpolation<Theme> }
    : {}
  : {} 

But since the InView component accepts props with the IntersectionObserverProps | PlainChildrenProps union type, the mentioned WithConditionalCSSProp type returns {} since the className props exists only in the PlainChildrenProps type.

To Reproduce

CodeSandbox

Expected behavior InView component types should be compatible with the @emotion/react

Forked On 27 Jul 2022 at 12:37:11

Thebuilder

Suppose it could make sense to make some better type inference based on the type of children. It might also be solved if the type for PlainChildrenProps extends a React.Component, instead of just HTMLProps. Kinda depends on where Emotion injects the css prop type.

Any reason you can't use the render props pattern to solve this issue?

Commented On 27 Jul 2022 at 12:37:11
Issue Comment

Thebuilder

Vitest runs tests 3x slower than Jest with threads: true (default) setting

Describe the bug

With the latest Vitest/Vite dependencies, Vitest is ~3 times slower than Jest in https://github.com/EvHaus/jest-vs-jasmine (more or less like a typical front-end repo) with the threads: true (default) setting.

Benchmark 2: yarn workspace jest test
  Time (mean ± σ):     34.704 s ±  2.548 s    [User: 91.246 s, System: 16.712 s]
  Range (min … max):   32.017 s … 41.447 s    10 runs
 
Benchmark 3: yarn workspace vitest test --threads=true
  Time (mean ± σ):     99.077 s ±  2.015 s    [User: 185.493 s, System: 40.303 s]
  Range (min … max):   97.352 s … 103.406 s    10 runs 

Related comment https://github.com/vitest-dev/vitest/issues/229#issuecomment-1003235680. That issue was closed but threads: true is still 3-4 slower than Jest after the issue was closed and the repro dependencies updated).

The issue was raised in Discord chat as well.

With threads: false Vitest is ~2 times faster, but the cost of it - there's no real isolation. With threads: false it's unfortunately not even close what Jest is doing in terms or resetting state, so not even sure if makes sense to compare threads: false with Jest (maybe threads: false should be compared with uvu or other "run-once-and-forget" runners with no real isolation). In a variety of projects that we have worked on, even with pure stateless components, the proper isolation is a super important concern. Especially in watch mode where introducing some unwanted state/mocking or polluting global/process state is simply an expected thing to happen due to the nature of various incremental changes (that are not always good or final) that simply break not properly isolated watch mode.

Reproduction

git clone https://github.com/EvHaus/jest-vs-jasmine 
yarn 

(if required, install hyperfine)

hyperfine --warmup 1 'yarn workspace jest test' 'yarn workspace vitest test --threads=true' 

System Info

System:
    OS: macOS 11.2.3
    CPU: (16) x64 Intel(R) Xeon(R) W-2140B CPU @ 3.20GHz
    Memory: 25.41 MB / 32.00 GB
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 16.13.1 - ~/.nvm/versions/node/v16.13.1/bin/node
    Yarn: 3.1.1 - ~/.yarn/bin/yarn
    npm: 8.1.2 - ~/.nvm/versions/node/v16.13.1/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  Browsers:
    Chrome: 97.0.4692.71
    Firefox Developer Edition: 61.0
    Safari: 14.0.3 

Used Package Manager

npm

Validations

Forked On 26 Jul 2022 at 09:25:46

Thebuilder

Is there anyway of debugging vitest performance? Maybe list what takes time on each run, or which dependencies are loaded? After switching a React Testing Library application (built with Vite) from Jest to Vitest, the test time CI (Azure DevOps) went from around 50s with code coverage using Jest, to 2 minutes without code coverage in Vitest. It could very well be related to dependencies we are using (like Material UI), but it's really hard to debug.

Commented On 26 Jul 2022 at 09:25:46

Thebuilder

`InView` component types are not compatible with the `@emotion/react`

Describe the bug

@emotion/react library allows to pass the css property into components if the className prop is a part of them. The InView has the className prop but typescript complains when passing the css prop.

@emotion/react types inject this property using the following type:

type WithConditionalCSSProp<P> = 'className' extends keyof P
  ? string extends P['className' & keyof P]
    ? { css?: Interpolation<Theme> }
    : {}
  : {} 

But since the InView component accepts props with the IntersectionObserverProps | PlainChildrenProps union type, the mentioned WithConditionalCSSProp type returns {} since the className props exists only in the PlainChildrenProps type.

To Reproduce

CodeSandbox

Expected behavior InView component types should be compatible with the @emotion/react

Forked On 26 Jul 2022 at 09:14:16

Thebuilder

Might you be able to solve this using a type guard?

Otherwise, how do you imagine the types being changed to support this pattern?

Commented On 26 Jul 2022 at 09:14:16

Thebuilder

chore: upgrade dependencies

Pushed On 22 Jul 2022 at 09:18:10

Thebuilder

Merge pull request #577 from thebuilder/chore/upgrade-deps

Pushed On 22 Jul 2022 at 09:18:10

Thebuilder

chore: upgrade dependencies

Created On 22 Jul 2022 at 09:18:09

Thebuilder

chore: upgrade dependencies

Created On 22 Jul 2022 at 07:40:39