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

Expose app global variable #6058

Closed
Chriscbr opened this issue Mar 25, 2024 · 8 comments · Fixed by #7078
Closed

Expose app global variable #6058

Chriscbr opened this issue Mar 25, 2024 · 8 comments · Fixed by #7078
Labels
🛠️ compiler Compiler ✨ enhancement New feature or request 📐 language-design Language architecture needs-discussion Further discussion is needed prior to impl Stale

Comments

@Chriscbr
Copy link
Contributor

Chriscbr commented Mar 25, 2024

Use Case

I've been thinking about this more, and I'm starting to feel like maybe it could make sense to expose app as a global variable as a substitute for this (added in #5594). Here's a design sketch:

// main.w

app; // std.IApp

class MyFactory {
  pub static makeBucket() {
    new cloud.Bucket() in app; // can be used as a scope
  }
}

// instead of util.env("WING_TARGET")
if app.platform == "sim" {
  log("app compiled to simulator");
} else {
  log("app compiled to another platform");
}

app can also be available in module files, but it would need a few restrictions (see below). It could be emitted in JavaScript as App.of(this).

// mod.w

class Store {
  new() {
    if app.platform == "sim" {
      // ...
    } else {
      // ...
    }
  }
  
  static myStaticMethod {
    app; // error: Cannot use `app` in static context
  }
}

class Subclass extends Store {
  new() {
    app; // error: Cannot access `app` in a subclass before super() has been called
    super();
  }
}

The motivation is a few-folded:

  • After we added feat(compiler): make "this" available in entrypoint files #5594 I started to think maybe "this" in entrypoint files could be confusing
  • "this" in entrypoint files is typed as IConstruct, but it would be more useful to have access to app-related facilities, like the compilation platform. Supporting both this and app might be more confusing though (not sure)

Proposed Solution

No response

Implementation Notes

No response

Component

Language Design

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.
@Chriscbr Chriscbr added ✨ enhancement New feature or request 📐 language-design Language architecture needs-discussion Further discussion is needed prior to impl labels Mar 25, 2024
@monadabot monadabot added this to Wing Mar 25, 2024
@github-project-automation github-project-automation bot moved this to 🆕 New - not properly defined in Wing Mar 25, 2024
@eladb
Copy link
Contributor

eladb commented Mar 25, 2024

I like the idea of exposing the app as a global (it's basically a sugar for nodeof(this).app) but:

  1. I am not sure I am comfortable snagging the symbol app into a keyword. But we can brainstorm names in #naming-things.
  2. I don't know that this is mutually exclusive to having this work in entrypoint code. I think we should keep this behavior (BTW, I am not sure this in the entrypoint and nodeof(this).app is the same thing.

@Chriscbr
Copy link
Contributor Author

I am not sure I am comfortable snagging the symbol app into a keyword. But we can brainstorm names in #naming-things.

My thinking was it could be a variable name instead of a dedicated keyword. So you can still use the name for other purposes, e.g.

let apps = ["app1", "app2"];
for app in apps {
  // app shadows over the global "app" variable
};

(BTW, I am not sure this in the entrypoint and nodeof(this).app is the same thing.)

Gotcha. I couldn't think off the top of my head how they'd be different, but we can let the idea bake and maybe something will come to light. Ultimately I think if we can simplify things it can be a good thing

@eladb
Copy link
Contributor

eladb commented Mar 26, 2024

it could be a variable name

Not sure it's super healthy for something like this to have double meanings. It's like you wouldn't want to be able to shadow this.

Maybe we can all it thisApp but it's ugly AF. Maybe thisapp?

@staycoolcall911 staycoolcall911 moved this from 🆕 New - not properly defined to 🤝 Backlog - handoff to owners in Wing Mar 28, 2024
@hasanaburayyan
Copy link
Contributor

Maybe we can all it thisApp but it's ugly AF. Maybe thisapp?

What about _app this conveys its an internal mechanism we could also go full Python and do __app__ 😁 (only kidding)

@yoav-steinberg
Copy link
Contributor

+1 for getting rid of this in entry point files, for non CDK users this outside of the context of an instance method just seems wrong. And thisapp or similar is much more expressive. Consider using this in some static method and then copying that method during a refactor into a module, it'll break, but thisapp won't.

@eladb
Copy link
Contributor

eladb commented Jun 6, 2024

Maybe @app?

Copy link

github-actions bot commented Sep 4, 2024

Hi,

This issue hasn't seen activity in 90 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days.
Feel free to re-open this issue when there's an update or relevant information to be added.
Thanks!

@github-actions github-actions bot added the Stale label Sep 4, 2024
@github-project-automation github-project-automation bot moved this from 🤝 Backlog - handoff to owners to ✅ Done in Wing Sep 6, 2024
@monadabot
Copy link
Contributor

Congrats! 🚀 This was released in Wing 0.84.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠️ compiler Compiler ✨ enhancement New feature or request 📐 language-design Language architecture needs-discussion Further discussion is needed prior to impl Stale
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

6 participants