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

@deephaven/utils - errors when installing in vscode extension #2185

Closed
bmingles opened this issue Aug 12, 2024 · 3 comments · Fixed by #2225
Closed

@deephaven/utils - errors when installing in vscode extension #2185

bmingles opened this issue Aug 12, 2024 · 3 comments · Fixed by #2225
Assignees
Labels
bug Something isn't working

Comments

@bmingles
Copy link
Contributor

I attempted to install @deephaven/utils in the vscode-deephaven extension and get a number of errors:
image

It looks like there are at least 2 classes of errors here:

  • Incompatible CustomEvent type (presumably due to differences in TypeScript version or tsconfig target)
  • References to jest. This dependency is not declared in package.json. We may also want to remove the dependency on jest from @deephaven/utils altogether since not all consumers would be using jest as their testing framework.
@mofojed
Copy link
Member

mofojed commented Sep 3, 2024

References to jest

We should move test code such as TestUtils into it's own directory, and possibly it's own package @deephaven/test-utils to lump a bunch of test utilities together.

@bmingles
Copy link
Contributor Author

bmingles commented Sep 3, 2024

References to jest

We should move test code such as TestUtils into it's own directory, and possibly it's own package @deephaven/test-utils to lump a bunch of test utilities together.

My vote would be a separate package since it's not guaranteed that consumers will use jest at all.

@mofojed
Copy link
Member

mofojed commented Sep 3, 2024

To clarify my previous comment... files that depend on jest or @testing-library/* or any of our other testing libraries/devDependencies, should not be included in the package build output and should be removed. It may make sense to move them all to a @deephaven/test-utils package (especially TestUtils, which is used by many of our other packages for testing), or simply moved to a folder that is excluded from build output (such as ChartTestUtils, which is not needed in any other package).

bmingles added a commit to bmingles/web-client-ui that referenced this issue Sep 17, 2024
bmingles added a commit to bmingles/web-client-ui that referenced this issue Sep 17, 2024
bmingles added a commit to bmingles/web-client-ui that referenced this issue Sep 17, 2024
bmingles added a commit to bmingles/web-client-ui that referenced this issue Sep 17, 2024
bmingles added a commit to bmingles/web-client-ui that referenced this issue Sep 17, 2024
bmingles added a commit to bmingles/web-client-ui that referenced this issue Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants