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

Can't find TextEncoder in tests after 2.2.2 release #60

Closed
karol-f opened this issue Mar 9, 2023 · 8 comments
Closed

Can't find TextEncoder in tests after 2.2.2 release #60

karol-f opened this issue Mar 9, 2023 · 8 comments

Comments

@karol-f
Copy link

karol-f commented Mar 9, 2023

Can't find TextEncoder in Jest tests after 2.2.2 release:

Test suite failed to run

    ReferenceError: TextEncoder is not defined

    > 1 | import { merge } from 'moderndash';

What is strange it want to get it from src folder:

at Object.<anonymous> (../../node_modules/moderndash/src/crypto/hash.ts:5:21)

I can see only src comment in the dist source code:

// src/crypto/hash.ts
var textEncoder = new TextEncoder();
@Maggi64
Copy link
Owner

Maggi64 commented Mar 9, 2023

Mhh TextEncoder should be a global and doesn't need an import.
https://developer.mozilla.org/en-US/docs/Web/API/TextEncoder?retiredLocale=de

On the first glance it looks like an issue outside of moderndash. It only appeared now in v2.2.2 because I moved the TextEncoder to the outer scope. Before it was generated in the hash function itself.

@karol-f can you give me a reproducible repo on stackblitz for example?
I will look at it there.

@Maggi64
Copy link
Owner

Maggi64 commented Mar 9, 2023

A google search showed me this, seems to be related: inrupt/solid-client-authn-js#1676

Seems like a jest issue. This probably fixes it for you: https://stackoverflow.com/a/68468204

@karol-f
Copy link
Author

karol-f commented Mar 9, 2023

Thanks for the hints, I will try to resolve it myself with e.g. @inrupt/jest-jsdom-polyfills package.

@karol-f
Copy link
Author

karol-f commented Mar 9, 2023

Importing @inrupt/jest-jsdom-polyfills in setupFilesAfterEnv: ['<rootDir>/jest-setup.ts'] file worked for me.

I just wonder if we really need TextEncoder in Node environment (e.g. in tests or Next.js on server-side).
This did a trick for me, works in both environments (when only importing merge from moderndash):

var textEncoder = typeof TextEncoder === 'function' ? new TextEncoder() : undefined;

Obviously, this second way will fail if you actually will use some method with TextEncoder usage, but at least it won't break every time in Node, without pollyfill.

@Maggi64
Copy link
Owner

Maggi64 commented Mar 9, 2023

Sounds good 👍 I might change the function to only create the TextEncoder object when actually needed.
I need to check benchmarks what impact this would have though.
Without tree-shaking (i.e. in tests) TextEncoder gets called without ever getting used.

@karol-f
Copy link
Author

karol-f commented Mar 9, 2023

Without tree-shaking (i.e. in tests) TextEncoder gets called without ever getting used.

I will just add that tree-shaking is usually done only in production builds (as it's computation heavy), so I assume that moderndash, in dev mode, will break also in e.g. Next.js as it does two separate builds - for the server and for the browser.

@Maggi64
Copy link
Owner

Maggi64 commented Mar 9, 2023

@karol-f Fixed in 991a1d9 v2.2.3

TextEncoder is only created when its needed in hash

You can close this issue if it works for you 👍

@karol-f karol-f closed this as completed Mar 9, 2023
@karol-f
Copy link
Author

karol-f commented Mar 9, 2023

I can confirm that it's working now :) Thank you!

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

No branches or pull requests

2 participants