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

Deno support #113

Closed
dfabulich opened this issue May 18, 2020 · 9 comments · Fixed by #140
Closed

Deno support #113

dfabulich opened this issue May 18, 2020 · 9 comments · Fixed by #140

Comments

@dfabulich
Copy link

To repro:

rm -rf denotest
mkdir denotest
cd denotest
git clone https://github.com/bikeshaving/crank.git
cd crank
npm i
npm run prebuild
npm run build
cd ..
cat <<EOF > test.ts
import {createElement} from './crank/src/index.ts';
import {renderer} from './crank/src/html.ts';

console.log(renderer.render(createElement('h1', {id: 'foo'}, 'Hello World'))
EOF
deno run test.ts

Actual:

Compile file:///private/tmp/denotest/test.ts
error: Uncaught NotFound: Cannot resolve module "file:///private/tmp/denotest/crank/src/events" from "file:///private/tmp/denotest/crank/src/index.ts"
@dfabulich dfabulich changed the title Crank doesn't work in deno Crank TS doesn't work in deno May 18, 2020
@dfabulich
Copy link
Author

dfabulich commented May 18, 2020

FWIW, this does work in deno:

import {createElement} from 'https://unpkg.com/@bikeshaving/crank@0.1.4/esm/index.js'
import {renderer} from 'https://unpkg.com/@bikeshaving/crank@0.1.4/esm/html.js';

console.log(renderer.render(createElement('h1', {id: 'foo'}, 'Hello world')));

… but I believe it's not consuming the types (or even the sourcemaps) that way.

@dfabulich
Copy link
Author

dfabulich commented May 18, 2020

denoland/deno#5308

You're going to love this: the best practice I can find is to write TSC-compatible TS (import './index')and transpile it to deno-compatible TS (import './index.ts').

Leadership!!!

@brainkim
Copy link
Member

brainkim commented May 19, 2020

Yeah, I think the path forward is to do like above and use something like unpkg and import the esm js artifacts. (Note that you should be importing the files in the root for maximum compatibility in 0.2.0. In case you’re interested, I’m the author of the following issue asking for guidance (denoland/deno#3196) but there doesn’t seem to be much movement on that front. The problem is that typescript forces you to not specify file extensions, while deno forces you to, and it seems like they’re at an impasse about this. I personally would prefer to have explicit file extensions and never understood the TS team’s rationale behind not allowing it, but whatever.

Something like this should work:

/// <reference types="https://unpkg.com/@bikeshaving/crank@0.1.4/index.d.ts" />
import {createElement} from 'https://unpkg.com/@bikeshaving/crank@0.1.4/index.js'
/// <reference types="https://unpkg.com/@bikeshaving/crank@0.1.4/html.d.ts" />
import {renderer} from 'https://unpkg.com/@bikeshaving/crank@0.1.4/html.js';

I’m not sure if they changed the API for the 1.0 release though. Searching the deno docs/github for “triple slash directive” should give you the latest information.

Thanks for looking into this stuff! Very curious about your thoughts on deno. I also think that the greenfield landscape of deno offers a huge opportunity in terms of evangelism, like if we can convince the deno community that Crank, and specifically its focus on async/generator functions is a perfect match for Deno’s API, which uses async iterators everywhere, we could jumpstart adoption.

@brainkim
Copy link
Member

https://github.com/denoland/deno/blob/45f9b32ef0416e0477e9f5335df49ca3cccdb6eb/docs/getting_started/typescript.md

This page seems to have more information. It looks like triple slash references are what we are supposed to do on our end as library authors, while the deno-types comments are what library consumers do.

@dfabulich
Copy link
Author

I'm feeling pretty "meh" on deno 1.0.

  1. Controlling security for the entire process with CLI flags is the wrong approach. Instead, I want individual modules to have permissions, so I can say "I depend on X but I don't want X to have any net/file access; I depend on Y which should have net access to example.com; I depend on Z which can open ports" etc. As it is, I have to depend on third-party code, and then run my entire process with the superset of all permissions any of them need. (I bet that browsers will do this, someday, and then deno will have to switch to that.)
  2. Importing URLs directly in the JS/TS is not great; habitually depending on deps.ts will make it harder to do static analysis. (How do you automatically flag known security issues when dependencies are just URLs?) Import maps are a better solution, but they're "unstable" in deno 1.0, and they still don't address the static analysis problem.
  3. Deno's approach to subresource integrity is a lock file, but lock files are disabled by default, which is a bad default. (Also, I predict that the browser people will cook up their own SRI metadata file, based around import maps, and then we'll have wanted Deno to use that, instead.)
  4. I want TS to work out of the box, but this isn't the way I want it, because it forces my entire dependency graph onto the same version of TS. I either want https://github.com/samuelgoto/proposal-pluggable-types or I want V8 to actually use TS types for performance optimizations. Running TSC at runtime is the worst way to make TS work out of the box at runtime.

@brainkim
Copy link
Member

Also, I predict that the browser people will cook up their own SRI metadata file, based around import maps, and then we'll have wanted Deno to use that, instead.

haha this is nightmare fuel.

I agree with all these points, but top-level await and a modern standard lib is kinda tempting.

@dfabulich
Copy link
Author

Funny you should mention it. Top-level await doesn't work in deno's REPL. denoland/deno#3700

TLA's just not ready yet. The V8 team is actively working on it. https://bugs.chromium.org/p/v8/issues/detail?id=9344

When they ship it, Node will use it, too. nodejs/node#31410

TLA is yet another area where deno's reach exceeded its grasp. What I feel like we learned from Node is that CLI JS has to stay one step behind the browsers. It's frustrating, because the browsers move so slowly, but getting out on the bleeding edge is just asking for incompatibility later down the road.

IMO, the only safe way to get ahead of the browsers is transpilation. Everything else has to wait.

As for modernizing the stdlib, I expect we'll see Node modernize its stdlib over time, e.g. fs/promises. https://nodejs.org/api/fs.html#fs_fs_promises_api

@brainkim
Copy link
Member

brainkim commented Jul 29, 2020

The following almost works.

server.tsx

/** @jsx createElement */
// @deno-types="https://unpkg.com/@bikeshaving/crank@0.3.0/index.d.ts"
import {createElement} from "https://unpkg.com/@bikeshaving/crank@0.3.0/index.js";
// @deno-types="https://unpkg.com/@bikeshaving/crank@0.3.0/html.d.ts"
import {renderer} from "https://unpkg.com/@bikeshaving/crank@0.3.0/html.js";


console.log(renderer.render(<div>Hello world</div>, document.body));

tsconfig.json

{
  "compilerOptions": {
    "target": "esnext",
    "module": "esnext",
    "lib": ["esnext", "dom"]
  }
}

command

deno run -c tsconfig.json server.tsx

I think the deno-types comment is kinda annoying and will see if we can get triple-slash reference comments in the d.ts files directly.

The code throws the following error from the HTML module.

[ERROR]: Invalid module name in augmentation. Module './index' resolves to an untyped module at 'https://unpkg.com/@bikeshaving/crank@0.3.0/index.js', which cannot be augmented.
declare module "./index" {

There is this module augmentation issue in Deno (denoland/deno#6839), but I don’t think declaring modules based on relative/absolute path is robust in any sense. I think the best thing to do is to declare a Crank global module and use that (similar to the way TypeScript uses the JSX module).

@brainkim brainkim changed the title Crank TS doesn't work in deno Deno support Jul 29, 2020
@brainkim brainkim mentioned this issue Jul 29, 2020
6 tasks
@brainkim brainkim reopened this Jul 30, 2020
@brainkim
Copy link
Member

brainkim commented Jul 30, 2020

Crank officially supports deno as of 0.3.1. You can import it as follows via unpkg.

/** @jsx createElement */
import {createElement} from "https://unpkg.com/@bikeshaving/crank@0.3.1/crank.js";
import {renderer} from "https://unpkg.com/@bikeshaving/crank@0.3.1/html.js";

console.log(renderer.render(<div>This is pretty cool</div>));

You need to make sure you add dom to the lib compiler option in tsconfig, or some typings will not work.

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 a pull request may close this issue.

2 participants