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

reduce node specific dependencies #1158

Closed
wants to merge 7 commits into from

Conversation

mash-graz
Copy link
Contributor

@mash-graz mash-graz commented Nov 6, 2023

Although most of the happy-dom works very well in node independent JS environments like in vite utilizing vite-plugin-node-polyfills I still had to modify the source code to work around two minor issues:

  1. The explicit import of performance from perf_hooks shouldn't be necessary in node >= v16 anymore, because it's now present in global scope out of the box. This eliminates the difference to WebAPI based solutions, where it is also globally provided as window.performance.

  2. The other obstacle concerns the isIP import from node:net. This is a somehow tricky issue, because most of node:net can't be substituted by polyfills and is therefore not supported in vite-plugin-node-polyfills and similar solutions. There are few npm packages available, which provide the isIP group of functions in a more or less compatible manner, but in the given case, where these routines are only called once, it's perhaps better to inline the necessary regex test patterns and avoid any additional dependency. That's the way suggested by this PR. For this purpose I have chosen a set of performance optimized regex patterns that is also faster than the implementation used in older node releases (see: net: improve performance of isIPv4 and isIPv6 nodejs/node#49568).

mash-graz added a commit to mash-graz/happy-dom that referenced this pull request Nov 6, 2023
mash-graz added a commit to mash-graz/happy-dom that referenced this pull request Nov 6, 2023
mash-graz added a commit to mash-graz/happy-dom that referenced this pull request Nov 6, 2023
mash-graz added a commit to mash-graz/happy-dom that referenced this pull request Nov 6, 2023
@mash-graz
Copy link
Contributor Author

mash-graz commented Nov 7, 2023

PS: removing the webcrypto -> crypto indirection resp. node:crypto dependency would also make some sense, because the commonly used polyfill solutions are incompatible with global-registrator in this case.

mash-graz added a commit to mash-graz/happy-dom that referenced this pull request Nov 7, 2023
mash-graz added a commit to mash-graz/happy-dom that referenced this pull request Nov 7, 2023
mash-graz added a commit to mash-graz/happy-dom that referenced this pull request Nov 7, 2023
@mash-graz
Copy link
Contributor Author

The mentioned break of global-registrator by polyfilling crypto is now also fixed.
It's using a dynamic import and redefinition of globalThis.crypto if really necessary (node v15-v19).

mash-graz added a commit to mash-graz/happy-dom that referenced this pull request Nov 7, 2023
@mash-graz
Copy link
Contributor Author

mash-graz commented Nov 7, 2023

Now I also removed the imports of TextDecoder from node:util.

There are just 3 explicit node dependency imports remaining:

  • url and buffer -- I simply left them, because of the overloading found in the code.

  • vm -- That's a more complicated case, because vm isolation in node and on the browser side looks utterly different. There is hardly any solution available, which would really work on both sides (perhaps some exotic WASM based execution engines, like componentize-js).

btw: The vm handling right now simply isn't compatible with available replacements (vm-browserify) because their createContext() mechanism doesn't work in the way chosen in:

VM.createContext(this);

You have to utilize their return value instead and take care of the prototype-chain, otherwise isContext() will not work in this simple replacement solutions.
But that's not an important issue, because polyfills do not make much sense here anyway.

@mash-graz
Copy link
Contributor Author

Adding another patch to fix issues concerning the absence of process.platform and process.arch in the userAgent string creation when used in vite environments.

This will otherwise immediately crash on Window() construction:

[WebServer] file:///home/mash/Projects/web-components/astro/astro-ce/dist/index-Ufypa1UF.js:34444
          userAgent: `Mozilla/5.0 (X11; ${R.process.platform.charAt(0).toUpperCase() + R.process.platform.slice(1) + " " + R.process.arch}) AppleWebKit/537.36 (KHTML, like Gecko) HappyDOM/${Zc.version}`
                                                             ^
TypeError: Cannot read properties of undefined (reading 'charAt')
``

@shirakaba
Copy link

shirakaba commented Nov 27, 2023

Was curious whether there’s a list anywhere detailing what the dependencies on Node (and Web) APIs are. I’d like to use happy-dom on JS engines like Hermes and QuickJS but will have to do a lot of trial-and-error to work out which things to cut out or polyfill.

@mash-graz
Copy link
Contributor Author

mash-graz commented Nov 28, 2023

Was curious whether there’s a list anywhere detailing what the dependencies on Node (and Web) APIs are.

Unfortunately I can't present you a satisfying solution for this problem. I also had to find it out by trial and error and used only the history remarks in the official node documentation and MDN pages as hint, which globals could be nowadays expected without explicit import and which operations should be available on node and the web platform by the same name and interface.

But I'm not a very experienced JS/TS developer -- To the contrary I'm just confronted with this issue, because I try to develop a new custom element SSG-rendering plugin for Astro, which provides better support for rust / WASM based components to finally get rid of LIT and other JS bound solutions.

In this plugin I'm providing the choice between three different DOM shims to the end user (happy-dom, linkedom, jsdom), because I'm still not sure, which of these alternatives should be seen as the most recommendable one. I therefore had to fight with all of them. jsdom is by far the most problematic, bloated, old-fashioned and slow on of this group. It's hardly useful anymore. happy-dom and linkedom are much more pleasant and play more or less in the same ballpark regarding most practical aspects. linkedom has even less problematic external dependencies and code incompatibilities compared to happy-dom, but this comes with the drawback of not always perfect standard compliant interfaces and less convincing continuous maintainer support. In my benchmarks measuring the rendering performance of simple custom elements happy-dom also seems to be in many cases slightly faster than linkedom to my surprise.

I hope, this practical observations help to find the best solutions for your own demands.

It's very likely that I'll have to add a few more patches to happy-dom to address runtime issues when used in vite environments. Nothing spectacular -- just to share feedback, minor improvements and development efforts with others.

@shirakaba
Copy link

shirakaba commented Dec 3, 2023

To assess which Node dependencies happy-dom relies upon, I performed a quick analysis of the packages used by searching for the RegEx /from '[A-z]/ under happy-dom/lib. It uses the following packages. Those outside of the node standard library, which may or may not depend upon Node themselves, I've marked in red:

buffer
child_process
console
crypto
- css.escape
- entities
fs
http
https
- iconv_lite
net
path
perf_hooks
stream
url
util
vm
- whatwg-encoding
- whatwg-mimetype
zlib

So although reducing Node.js dependencies may be achievable, disentangling happy-dom from Node.js altogether may be impractical.

Given I'd like to use this on the Hermes JS engine, I'd personally benefit from a way to set up "as much of DOM as possible without bringing in Node" (I only need enough DOM to make React happy; no need for APIs like url, vm, and child_process), but think it would require a lot of will from the maintainer to make any progress on.

In the Rust/Cargo ecosystem, it's possible to customise packages by specifying features, which would be perfect for something like this, but there's not really an idiomatic way to do that on npm 😕

Lacking that, we could consider excising the dependencies using a bundler (in which case it could become a consumer concern rather than a happy-dom concern). My dominimal package does something similar, stripping out superfluous features of JSDOM by replacing them with either no-op modules or polyfills via Webpack, to focus just on DOM and not the whole web platform – but it does ultimately still depend upon Node.js.

The same could be done for happy-dom, though it's tricky to set up (and annoying to maintain if the repo keeps being refactored), so I'm not too keen on it.

mash-graz added a commit to mash-graz/happy-dom that referenced this pull request Dec 4, 2023
mash-graz added a commit to mash-graz/happy-dom that referenced this pull request Dec 4, 2023
Comment on lines 135 to 141
if (
!('crypto' in globalThis) ||
Object.getPrototypeOf(globalThis.crypto) === Object.getPrototypeOf({})
) {
globalThis['crypto'] = import('crypto').then((c) => c.webcrypto);
}

Copy link

@shirakaba shirakaba Dec 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should avoid dynamic imports as it creates new problems.

Also in any case, we'd need to await this Promise, as right now the value of globalThis.crypto will be Promise {status: "resolved", result: ()} (i.e. a resolved Promise wrapping the value of webcrypto).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this feedback!
I'll look for a better replacement...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's now solved in a slightly modified manner:

async function webcryptoImportFallback() {
	if (
		!('crypto' in globalThis) ||
		Object.getPrototypeOf(globalThis.crypto) === Object.getPrototypeOf({})
	) {
		// Import required only on node < 19
		globalThis['crypto'] = (await import('crypto')).webcrypto;
	}
}
webcryptoImportFallback();

A rather verbose variant to work around the prohibited top-level-await but still using dynamic import.
But the import should be only required for node < 19. In this particular case it will just have the same effect as in the original source code. But on most JS runtimes (more actual node releases / deno / browsers) webcrypto is already accessible as globalThis.crypto without any explicit import.

We could avoid the dynamic import fallback by always importing * from 'crypto' and handle the conditional redirection later if needed, but it again wouldn't work with commonly used polyfill solutions out of the box.

Copy link

@shirakaba shirakaba Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed for Node, it'll only be a problem on older environments, but even then, I'm not sure about this workaround. This will queue a microtask to polyfill globalThis.cypto, but as the surrounding code is synchronous, something could still try to access that property before it's ready.

To avoid this weak point, I think ideally polyfilling crypto should be a responsibility of the user rather than the library, so that they can safely coordinate the polyfill flow (if needed at all) in their userland code.

  • If using a bundler: use resolve.alias (or similar) to alias crypto to a local polyfill of webcrypto.
  • If not using a bundler: in package.json, alias the Node SDK dependency to a local polyfill of webcrypto:
      // ...
      "dependencies": {
        "crypto": "file:./lib/web-crypto-polyfill@^1.0.0",
      }
    }
    ... where web-crypto-polyfill would be an npm package that pretty much just does this in its entrypoint JS file:
    import { webcrypto } from 'crypto';
    globalThis.crypto = webcrypto;

Copy link
Contributor Author

@mash-graz mash-graz Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really appreciate your critical review of the suggested code changes, but honestly I can't agree with your point of view in some significant aspects.

The webcrypto functions are in fact not needed by any internal processing in happy-dom. They are just reexported for WebAPI completeness as requested by #1050.

Right now it's handling this job in a strict node-centric manner, which unfortunately isn't compatible with other JS runtimes.

For example in deno the node:crypt module isn't available at all and any import attempt throws an error. And in many common tools for automatic polyfilling node specific interfaces, like the vite-plugin-node-polyfills used in my particular case, crypto imports just get shimmed by empty objects. The documentation of these helpers often explicitly advice to avoid the incomplete dummy replacement in this particular case (for example: here).

As a consequence of this lack of support under many circumstances, we are just shifting the issue from one level to the next one as long as we don't try to avoid this node specific imports wherever possible resp. replace them by alternative solutions and dynamic imports, which will be only activated in case of actually running on older node environments.

If we can't figure out a more satisfying and user-friendly solution, I would even suggest removing the crypto reexport again, which would at least eliminate the need of additional polyfills as long as this interface isn't actually required by an application.

Nevertheless, I think, my suggested workaround could be seen as an acceptable compromise to minimize these compatibility issues in a more desirable manner.

But if you see any further improvements to process the async dynamic import in more proper fashion (e.g., in regard to the type definition in IWindow), I would be really happy to listen to your advice.

@capricorn86
Copy link
Owner

Hi @shirakaba and @mash-graz! 🙂

Sorry for taking such long time before answering. I will look into this now.

@capricorn86
Copy link
Owner

capricorn86 commented Jan 15, 2024

@mash-graz and @shirakaba I have looked into this a bit more now. I see two solutions.

Alternative 1 - New Window class
Create a new Window class that has no dependencies to Node.js. The other Window classes can extend from it.

Window <= BrowserWindow <= BrowserWindowWithoutNodeJS

BrowserWindowWithoutNodeJS will lack some functionality, such as support for fetch(). For some functionality it will be possible to mock the functionality.

Alternative 2 - Bundle
We create a new package (e.g. "happy-dom-browserified"). The package contains a bundle or just a copy of "happy-dom" with references to Node.js replaced with a polyfill.

Alternative 2 is probably easier, but Alternative 1 has the benefit of separating Node.js dependencies in the code, which can become useful in the future.

@mash-graz
Copy link
Contributor Author

mash-graz commented Jan 16, 2024

Thanks for taking a look and reflecting this question.

Alternative 1 - New Window class Create a new Window class that has no dependencies to Node.js. The other Window classes can extend from it.

That's perhaps the most promising solution to manage reduced dependency sets in JS/TS, where we don't have more suitable mechanisms like C/C++ preprocessors or rust feature flags.

I partly used this strategy already, because you can't use the Window class and its VM isolation in more limited environments. In this case you have to choose the GlobalWindow subset as a more compatible starting point.

But VM dependency is a very special case, which can't be replaced easily. The majority of node specific imports are much simpler to avoid/replace, but they are again spread all over the place. I don't know if it makes more sense to just avoid/replace them wherever possible or indeed pack them in a dedicated class extension?

I also have my doubts, if it's really worth to spend lots of efforts on this feature?
jsdom removed its browserify variant again after a while because oft its maintenance burden.
linkedom works, but it simply doesn't provide VM isolation at all...
But perhaps you can figure out a more satisfying solution/compromise for this puzzling challenge.

@shirakaba
Copy link

shirakaba commented Jan 17, 2024

@capricorn86 I strongly support Alternative 1. Would provide a great amount of value.

I actually made a fork called unhappy-dom which strips out all the Node.js usages (yes, sacrificing a lot of functionality), but it did involve a fair bit of surgery – I think there’s no practical way to do it with bundler tricks alone, especially as polyfills often assume the presence of APIs from some other environment that might equally be unsupported.

I’ll share unhappy-dom later for reference to show what I ended up stripping out – just can’t get to it right now as I’m travelling and I didn’t get round to pushing my commits yet.

@capricorn86
Copy link
Owner

capricorn86 commented Jan 29, 2024

@mash-graz and @shirakaba I have created a new repository called happy-dom-without-node.

Installation

npm install happy-dom-without-node

Usage

import { Window } from 'happy-dom-without-node';

const window = new Window({ url: 'https://localhost:8080' });
const document = window.document;

document.body.innerHTML = '<div class="container"></div>';

const container = document.querySelector('.container');
const button = document.createElement('button');

container.appendChild(button);

// Outputs "<div class="container"><button></button></div>"
console.log(document.body.innerHTML);

I think this is the best we can do for now. I will close this PR.

@shirakaba
Copy link

@capricorn86 Thanks so much! I'm confused as to the setup, though – have some files not been committed to the repo? I don't see where it exports Window from, for example.

@capricorn86
Copy link
Owner

capricorn86 commented Jan 29, 2024

@capricorn86 Thanks so much! I'm confused as to the setup, though – have some files not been committed to the repo? I don't see where it exports Window from, for example.

It extracts the code from the happy-dom NPM package and polyfills it when running npm run build.

There is a cron job in place that will keep the happy-dom-without-node package up to date by checking if a new version of happy-dom has been released. It will then re-build and publish itself to NPM. It will also do type checking and run e2e tests to make sure that it will keep on working.

@shirakaba
Copy link

I see, thanks. It sounds like a Browserify kinda thing, then – will allow Happy DOM to be run in browser environments, which those polyfills generally assume.

Unfortunately, NativeScript is more like a Web Worker than a browser environment, so lacks the APIs the polyfills build upon, but fortunately I have since learned of worker-dom which covers my use-case, so I’m set.

@capricorn86
Copy link
Owner

@shirakaba it is not using Browserify. It replaces with dependencies with custom made polyfills that doesn't depend on Node.js.

The polyfills can be found here:
https://github.com/capricorn86/happy-dom-without-node/tree/main/polyfills

It doesn't rely on a Browser environment. It is just using a browser for performing E2E tests. However, some functionality will not work (like fetch).

Worker DOM seems like an interesting project.

@shirakaba
Copy link

shirakaba commented Jan 30, 2024

I mean that it's doing the same kinda thing as Browserify – polyfilling Node.js SDK dependencies with equivalents implemented using functionality usually found in a browser environment (e.g. Streams, Crypto, and inside whatwg-url, TextEncoder and TextDecoder), though now becoming more of a WinterCG environment.

But looking closer, if it's only those, NativeScript could probably cover them, as we're working on WinterCG 👍

EDIT: for the sake of illustration, I've pushed the now fairly outdated unhappy-dom so you can see what I was on about. I just ripped out any parts depending on Node.js to leave mostly just DOM tree manipulation. It may be an obscure use-case, though, so I think I'll continue with worker-dom which was designed for Web Workers from the beginning.

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

Successfully merging this pull request may close these issues.

3 participants