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

Eliminate the Environment class #59

Merged
merged 9 commits into from
Mar 17, 2024
Merged

Eliminate the Environment class #59

merged 9 commits into from
Mar 17, 2024

Conversation

dfreeman
Copy link
Member

Background

This PR is a followup to #58. As promised there, it eliminates the need to use get when working directly with the @env argument. It does this by largely getting rid of the explicit notion of a special Environment wrapper type.

This Change

Instead of handing components in the UI an Environment object wrapped around whatever @env was passed in, we now just directly use the given @env value as much as possible. Since there were previously several public methods you could call on an Environment object, this is another breaking change that will require some (hopefully not too obtrusive) updates in consumers:

  • The functionality previously provided by @env.metaForField has been eliminated. The only real use case we ever actually had for that was determining the canoncal location in the env where a trail of $binds actually pointed, which is now possible via import { resolveEnvPath } from 'ember-exclaim'.
  • Creating child environments via @env.extend is no longer directly possible. Previously, you would call this.args.env.extend(foo) and then pass the resulting value as the second parameter to {{yield}}. Now, you pass whatever foo is directly as the second {{yield}} param, and the details of adding those keys to the ambient environment for your child are internal to Exclaim. In general, this should just allow components that expose bound values to their children to simplify their implementation slightly and otherwise be unimpacted.

Not This Change

As in #58, looking through this diff may suggest obvious additional work we should take on. The following are things this PR intentionally does not do:

  • Update the GLOSSARY document to reflect the new state of things. This will happen, just not yet.
  • Modernize the implementation of components (usage of @ember/component, @computed, etc) in Exclaim itself or in the playground app.
  • Add an opt-in flag on ExclaimUi that uses unadorned getters for binding instead of computed() for compatibility with @tracked data. That may or may not happen before 2.0, but the idea would be that this opt-in mode becomes the only mode in 3.0 and acts as a path to incremental migration to @tracked in 2.x.
  • TypeScript. At a minimum for 2.0 I want to add canonical declarations here so we stop writing piecemeal ones everywhere, but it may make sense (we'll see after the refactoring dust settles) to actually just convert the source over; we'll see.

Notes/Caveats

In trying to track down what ultimately turned out to be 100f0e7, I refreshed the v2 addon blueprint in this repo to make sure I was getting the latest version of various build dependencies. You should be able to largely ignore that commit and just look through the others one by one.

@dfreeman dfreeman merged commit 66c5d30 into main Mar 17, 2024
8 checks passed
@dfreeman dfreeman deleted the drop-env-wrapper branch March 18, 2024 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants