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

Lifting files and directories #5449

Open
eladb opened this issue Jan 16, 2024 · 21 comments
Open

Lifting files and directories #5449

eladb opened this issue Jan 16, 2024 · 21 comments
Labels
✨ enhancement New feature or request 📐 language-design Language architecture needs-discussion Further discussion is needed prior to impl

Comments

@eladb
Copy link
Contributor

eladb commented Jan 16, 2024

Use Case

I am looking for a way to reference files or directories (relative to source file).

Proposed Solution

Latest and greatest:

bring fs;

// file (which is also a directory) or path, std.File or std.Path
let public = file!("./public");

test "print the index file" {
  // public is now just a string, or could be an inflight client with a .path property
  // It's a (new) absolute path that the lifter has taken care getting into the runtime for you
  let index = fs.readFile("{public}/index.html");
  log(index);
}

test "print all files" {
  for f in fs.readdir(public) {
    // The nice part about this is that any fs operations should still work as expected
    let data = fs.readFile("{public}/{f}");
    log(f);
  }
}

Component

Language Design, Compiler

Community Notes

  • Please vote by adding a 👍 reaction to the issue to help us prioritize.
  • If you are interested to work on this issue, please leave a comment.
  • If this issue is labeled needs-discussion, it means the spec has not been finalized yet. Please reach out on the #dev channel in the Wing Slack.
@eladb eladb added ✨ enhancement New feature or request needs-discussion Further discussion is needed prior to impl labels Jan 16, 2024
@eladb
Copy link
Contributor Author

eladb commented Jan 16, 2024

BTW, I think this might be a reasonable and simple solution until we have something more formal for #3736

@Chriscbr
Copy link
Contributor

@skyrpex
Copy link
Contributor

skyrpex commented Jan 16, 2024

I didn't know that, Chris! I prefer something along the modern syntax.

@eladb
Copy link
Contributor Author

eladb commented Jan 17, 2024

@skyrpex any suggestions?

@staycoolcall911 staycoolcall911 added the 📐 language-design Language architecture label Jan 24, 2024
@skyrpex
Copy link
Contributor

skyrpex commented Feb 16, 2024

What about a syntax similar to ESM's import.meta, but with wing flavour?

For example, bring.meta.filename and bring.meta.dirname.

@yoav-steinberg
Copy link
Collaborator

yoav-steinberg commented Feb 17, 2024

I'm trying to better understand this:

  • In preflight __dirname should just return the dir name of the file. I guess this should and can work (with extern). Doesn't it?
  • In inflight, what do we want? The dir name of the wing source file (same as preflight) or the runtime dirname (like node)?
    • if we want the location of wing source file, there's no issue, same as preflight.
    • if we want the runtime location, I think we're doing something wrong since this might not even be a file system. What does __dirname in JS do when running on a browser? Can you elaborate why this is useful?

If we don't care about runtime location, just build time location then I'm for just using __dirname, __filename (and __linenum and __columnnum). I think that's the best DX, even though I think it's pretty ugly.

@eladb
Copy link
Contributor Author

eladb commented Feb 18, 2024

The location is actually not the important piece (at least in most cases).

If we go back to the fundamentals. What's the use case for __dirname? It's basically accessing assets (files and directories) that are part of the source tree.

This use case is relevant both for inflight and preflight of course (e.g. I want to bundle a .png as part of my inflight and use it).

I am wondering if perhaps we need something a bit more higher level that will allow the compiler/bundler to do the right thing and package the assets into the relevant runtime context.

Maybe something like:

// this returns an `std.Asset` that represents the entire directory.
// "asset" is a builtin because the path it takes is relative to the *source file*.
let public = asset("./public");

test "print the index file" {
  let index = public.readFile("index.html");
  log(index);
}

test "print all files" {
  for f in public.readdir() {
    log(f);
  }
}

What do you all think?

@MarkMcCulloh
Copy link
Contributor

I like the asset idea and the solution probably has to be something like that (preflight relative path thing that binds into inflight with a way to access the data).

I think it's unfortunate that we'd basically be duplicating the fs API on this though, and limiting to directories seems unnecessary. What if we do keep this scoped to paths and say "if you reference the lifted path inflight, it will exist"

bring fs;

// file (which is also a directory) or path, std.File or std.Path
let public = file("./public");

test "print the index file" {
  // public is now just a string, or could be an inflight client with a .path property
  // It's a (new) absolute path that the lifter has taken care getting into the runtime for you
  let index = fs.readFile("{public}/index.html");
  log(index);
}

test "print all files" {
  for f in fs.readdir(public) {
    // The nice part about this is that any fs operations should still work as expected
    let data = fs.readFile("{public}/{f}");
    log(f);
  }
}

@eladb
Copy link
Contributor Author

eladb commented Feb 18, 2024

@MarkMcCulloh I like this direction. It does imply that we won't be able to do any optimizations (i.e. just lift a single file instead of a directory) but it sounds like a premature optimization.

In a sense, this is basically the use case of "lifting files" into inflight, which is not the same as referencing files in the project directory during preflight.

I'll split into two issues.

@eladb eladb changed the title Reference a source-adjacent paths (__dirname) Reference a source-adjacent paths in preflight (__dirname) Feb 18, 2024
@eladb eladb changed the title Reference a source-adjacent paths in preflight (__dirname) Referencing source-adjacent paths in preflight (__dirname) Feb 18, 2024
@MarkMcCulloh
Copy link
Contributor

MarkMcCulloh commented Feb 18, 2024

I like this direction. It does imply that we won't be able to do any optimizations (i.e. just lift a single file instead of a directory) but it sounds like a premature optimization.

It's not part of the API but the user does still have the single-file option since there's no reason this can't work for individual files too.
You're right though, I think the optimizations available to a "lazy-phantom filesystem" with asset() would be very interesting. Strong OCI vibes.

In a sense, this is basically the use case of "lifting files" into inflight, which is not the same as referencing files in the project directory during preflight.

The proposed solution of assert()/file() covers both cases, no? As far as a user is concerned I don't see the difference between those cases.

@yoav-steinberg
Copy link
Collaborator

yoav-steinberg commented Feb 18, 2024

I like these ideas.
Note that in many cases a better dx would be if file/asset were more like keywords expecting a string literal and not builtins expecting a string type. That way you can get IDE path autocomplete and squiggly red line in case the path is wrong (like what we have for extern "file.js"):

let public = asset "./public";
                //  ^^^^^ path "./public" not found in current_file_name.w's path

@MarkMcCulloh
Copy link
Contributor

MarkMcCulloh commented Feb 18, 2024

That way you can get IDE path autocomplete and squiggly red line in case the path is wrong

IDE path completion doesn't require special syntax, right now it's sufficient to be in a string and start with ./. Special syntax would avoid the need for that trigger though.

Also, unlike extern, these paths will probably be resolved during synthesis (or one of the hooks), so users will not see any squiggles in the IDE without a full wing compile. IMO I wouldn't want them to resolve before that because preflight itself could be building these files.

I'm not against special syntax, I think it'd be pretty cool, just want to make sure we choose it for a good reason.

@eladb
Copy link
Contributor Author

eladb commented Feb 19, 2024

I am repurposing this issue to focus on lifting files and directories instead of __dirname because that's actually much more interesting and the discussion here is focused on this.

@eladb eladb changed the title Referencing source-adjacent paths in preflight (__dirname) Lifting files and directories Feb 19, 2024
@eladb
Copy link
Contributor Author

eladb commented Feb 19, 2024

Y'all know I think we made a mistake with extern "foo"...

Maybe all these global builtins function-like should have some mark:

  1. extern!(), file!(), assert!()
  2. ~assert()
  3. file~("./path")
  4. $file("./path")
  5. #file("./path");
  6. "file#("./path");`
  7. _file("./path");

The ! suffix feels cleanest to me.

@skyrpex
Copy link
Contributor

skyrpex commented Feb 19, 2024

Interesting discussion. So basically, file("./path") will bundle a directory or file during preflight, and make it available during inflight.

Reminds me of the different asset handling techniques in bundlers. For example, see Vite's Static Asset Handling. You can import an image from a JavaScript file and what you get is the final URL:

import imgUrl from './img.png'
document.getElementById('hero-img').src = imgUrl

You can also use import suffixes to customize what's being imported:

import workletURL from 'extra-scalloped-border/worklet.js?url'
CSS.paintWorklet.addModule(workletURL)

or

import shaderString from './shader.glsl?raw'

@MarkMcCulloh
Copy link
Contributor

Y'all know I think we made a mistake with extern "foo"

The distinction with extern is that it's part of a declaration, not a plain expression. This kinda shows how the different syntax is useful, because the expectation of what extern does WRT outside files should be different than file() with the currently implementation.

If the mistake with extern is that it's declarative, then fair enough. I do think there should be some fundamental rework of extern, but the problem isn't just if it looks like extern "foo" or extern("foo").

Maybe all these global builtins function-like should have some mark

This was discussed before when the first globals were added. IMO a marker implies these things are special, but they're just global. file is not functionally different than user-defined functions/closures. Also to be consistent we'd have to have log!() too, which seems off,

@Chriscbr
Copy link
Contributor

Chriscbr commented Feb 20, 2024

file is not functionally different than user-defined functions/closures.

I guess the difference is that the string passed to file is evaluated with respect to the current source file, no? It sounds like that's the motivation for a dedicated syntax (like Yoav suggested)

If it were a plain function, then it could be used as a value (passed to higher order functions, etc) and would produce the same asset when called in different directories with the same path argument, which sounds like it isn't the case.

// dir1/mod.w
let a = asset "./flower.png"; // dir1/flower.png

// dir2/mod.w
let a = asset "./flower.png"; // dir2/flower.png

It seems like the function is doing something special that can't be achieved in Wing userland

@MarkMcCulloh
Copy link
Contributor

It seems like the function is doing something special that can't be achieved in Wing userland

That's fair, I was more arguing against the ! marker concept for globals like assert/log. "Special" here is subjective.

I do still think treating file like a regular function is feasible, akin to CJS require, which can work as expected even when passing the function around.

@eladb
Copy link
Contributor Author

eladb commented Feb 21, 2024

I guess the difference is that the string passed to file is evaluated with respect to the current source file, no? It sounds like that's the motivation for a dedicated syntax (like Yoav suggested)

Yes, that's why I think it must be a special thing.

Another way to think about it is to think about it as a primitive type (file):

let myFile: file = f"./my-file";

myFile.read();
myFile.path
myFile.extname
myFile.dirname

The compiler compiles this to:

const myFile = new std.File(path.join(__dirname, "./my-file"));

Or something like this.

What do y'all think? I kind of like the idea of a file being a primitive type.

@Chriscbr
Copy link
Contributor

Chriscbr commented Apr 1, 2024

Related: #6108

@ekeren
Copy link
Collaborator

ekeren commented May 2, 2024

Also Related: #6380

(TSOA resource)

@MarkMcCulloh MarkMcCulloh added this to the Winglang Stable Release milestone May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request 📐 language-design Language architecture needs-discussion Further discussion is needed prior to impl
Projects
Status: 🤝 Backlog - handoff to owners
Development

No branches or pull requests

7 participants