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

JSDOM port #542

Closed
wants to merge 9 commits into from
Closed

JSDOM port #542

wants to merge 9 commits into from

Conversation

MarkTiedemann
Copy link
Contributor

@MarkTiedemann MarkTiedemann commented Jul 20, 2019

Closes #537.

$ cd jsdom
$ npm install --no-package-lock [email protected] [email protected] [email protected]
$ ./node_modules/.bin/webpack-cli
Hash: 80cb94c06fc1cfb9a48b
Version: webpack 4.36.1
Time: 2422ms
Built at: 07/20/2019 11:51:39 PM
     Asset      Size  Chunks                    Chunk Names
./jsdom.js  3.05 MiB       0  [emitted]  [big]  main
Entrypoint main [big] = ./jsdom.js
 [17] (webpack)/buildin/global.js 472 bytes {0} [built]
[108] (webpack)/buildin/module.js 497 bytes {0} [built]
[231] canvas (ignored) 15 bytes {0} [built]
[440] ./streams (ignored) 15 bytes {0} [built]
[441] ./extend-node (ignored) 15 bytes {0} [built]
[519] util (ignored) 15 bytes {0} [built]
[521] util (ignored) 15 bytes {0} [built]
[562] buffer (ignored) 15 bytes {0} [optional] [built]
[563] crypto (ignored) 15 bytes {0} [optional] [built]
[625] crypto (ignored) 15 bytes {0} [built]
    + 948 hidden modules
$ deno jsdom.ts
Compile file:///Users/marktiedemann/dev/deno_std/jsdom/jsdom.ts
Hello, Deno!

TODOs:

  • Add preliminary types.
  • Add preliminary tests.
  • Write documentation.

Looking for feedback.

cc @ry

jsdom/jsdom.ts Outdated Show resolved Hide resolved
jsdom/jsdom.ts Outdated Show resolved Hide resolved
@MarkTiedemann MarkTiedemann changed the title WIP: JSDOM port JSDOM port Jul 25, 2019
@MarkTiedemann
Copy link
Contributor Author

I think this is ready to be reviewed now, @ry.

One major painpoint, I guess, is that the DOM and JSDOM types are still preliminary. There are also some other future TODOs, for example, investigating whether the bundle size can be improved, testing more of JSDOM's functionality, and rewriting/wrapping some Node specific-parts in JSDOM (for example, JSDOM.fromURL and JSDOM.fromFile both use built-in Node modules). But I think these can be tackled in future PRs.

import { jsdom as instance } from "./vendor/jsdom.js";
import { jsdom } from "./types/jsdom.ts";

export const JSDOM = instance.JSDOM as jsdom.JSDOM;
Copy link

Choose a reason for hiding this comment

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

Another implementation of this that should also work:

export { jsdom as JSDOM } from "./vendor/jsdom.js";
export { jsdom as JSDOM } from "./types/jsdom.ts";

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 the suggestion, @j-f1. Unfortunately, this doesn't work on my machine. When I change it, JSDOM can't be imported anymore.

$ deno test.ts
Compile file:///Users/marktiedemann/dev/deno_std/jsdom/mod.ts
error: Uncaught SyntaxError: The requested module './mod.ts' does not provide an export named 'JSDOM'

Copy link

Choose a reason for hiding this comment

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

Well, that’s unexpected. Sorry to send you on a wild goose chase!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries, @j-f1.

I just tried again, with the same code, and got a different error:

$ deno test.ts
Compile file:///Users/marktiedemann/dev/deno_std/jsdom/mod.ts

error TS2300: Duplicate identifier 'JSDOM'.

► file:///Users/marktiedemann/dev/deno_std/jsdom/mod.ts:1:19

1 export { jsdom as JSDOM } from "./vendor/jsdom.js";
                    ~~~~~

error TS2300: Duplicate identifier 'JSDOM'.

► file:///Users/marktiedemann/dev/deno_std/jsdom/mod.ts:2:19

2 export { jsdom as JSDOM } from "./types/jsdom.ts";
                    ~~~~~


Found 2 errors.

This seems like a more expected error, still unfortunate.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect denoland/deno#2746 will fix this (or at least make it possible).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kitsonk, awesome! As soon as denoland/deno#2746 is released, I will update the types here.

Copy link
Contributor

Choose a reason for hiding this comment

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

.d.ts support is in 0.16.

@ry
Copy link
Member

ry commented Aug 23, 2019

I'm having trouble running the tests with the big bundle file...

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.

wanted: JSDOM port
6 participants