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

Rethink how JavaScript packages are generated #176

Open
iacore opened this issue Jan 25, 2023 · 5 comments
Open

Rethink how JavaScript packages are generated #176

iacore opened this issue Jan 25, 2023 · 5 comments
Labels
discuss Ideas that might not make the cut

Comments

@iacore
Copy link

iacore commented Jan 25, 2023

Currently, JS files produced by Ditto rely on npm for linking. The reason in that they import each other with import std/...`.

It might be better to generate both packages and dist in the some folder, and use relative imports. This way, these files can be served directly to be used in browser, or in Deno.

@jmackie
Copy link
Member

jmackie commented Jan 25, 2023

Hey @locriacyber 👋 Thanks for taking an interest in ditto!

JS files produced by Ditto rely on npm for linking

This was a deliberate design choice, as I wanted ditto packages to be able to express NPM dependencies.

See https://github.com/ditto-lang/vdom/blob/702d6e4c1bb711b11ca6a0bc3bafc19d8fdfdb3c/ditto.toml#L10-L11 for example: the vdom package depends on snabbdom. When you run npm install not only will it link everything in packages/*, it will also pick up and install these dependencies.

It might be better to generate both packages and dist in the some folder, and use relative imports.

This is, in effect, what PureScript does. And it has this issue where NPM dependencies are implicit. In cases like purescript-react-basic those dependencies are more obvious, but there are some where it can be an annoying gotcha.

Re serving generated files directly to the browser: I had imagined ditto code would always be passed through a bundler, but this would be a nice thing to support. I don't think it would work given the presence of NPM dependencies though?

And WRT Deno: I plan to support this as a separate codegen target in the future, bear with me.

@iacore
Copy link
Author

iacore commented Jan 25, 2023

In Deno, you can import npm modules with npm:snabbdom. The alternative is to generate import_map.json, which is like package.json but for Deno.

@iacore
Copy link
Author

iacore commented Jan 25, 2023

When you run npm install not only will it link everything in packages/*, it will also pick up and install these dependencies.

See ditto-lang/todomvc#3. It doesn't work with pnpm.

@iacore
Copy link
Author

iacore commented Jan 25, 2023

Also, do you have other community channels, like Matrix? I'm interested in talking to you about ditto.

@jmackie
Copy link
Member

jmackie commented Jan 25, 2023

In Deno, you can import npm modules with npm:snabbdom. The alternative is to generate import_map.json, which is like package.json but for Deno.

Sweet, good to know 🙏 But like I say, the current JavaScript code generator isn't intended to be deno compatible. My plan was to introduce a new target for deno (#15 (comment))

See ditto-lang/todomvc#3. It doesn't work with pnpm.

Yeah that's a good point. The root package.json isn't really within ditto's control though (beyond bootstrapping a new project with ditto bootstrap --js) so I'm happy to leave that to users to tweak according to their preferred package manager.

I might need to rethink how the packages/*/package.json files get generated to ensure compatibility with the different package managers though 🤔

fn run_package_json(input: &str, output: &str) -> Result<()> {
use serde_json::{json, Map, Value};
let config = read_config(input)?;
// https://stackoverflow.com/a/68558580/17263155
let value = json!({
"name": config.name.into_string(),
"type": "module",
"dependencies": config
.dependencies
.into_iter()
.map(|name| (name.into_string(), String::from("*")))
.collect::<HashMap<_, _>>(),
});
let mut object = if let Value::Object(object) = value {
object
} else {
// Look at the macro call, it's an object.
unreachable!()
};
if let Some(additions) = config.codegen_js_config.package_json_additions {
// NOTE "name" and "type" can't be overriden
object = merge_objects(additions, object)
}
let file = File::create(output).into_diagnostic()?;
return serde_json::to_writer(file, &object).into_diagnostic();

Also, do you have other community channels, like Matrix? I'm interested in talking to you about ditto.

There's no community at the moment 😅 Would just be me in the channel...

If you wanted to set something up I'd be grateful. Although I have a slight preference for using discussions.

@jmackie jmackie added the discuss Ideas that might not make the cut label Jan 25, 2023
@jmackie jmackie changed the title Produce ES Modules Rethink how JavaScript packages are generated Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Ideas that might not make the cut
Projects
None yet
Development

No branches or pull requests

2 participants