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

Make hash built-in in strict-mode #999

Merged
merged 7 commits into from
Jun 12, 2024

Conversation

NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Dec 22, 2023

Propose making (hash) a built in helper

Rendered

Summary

Today, we need

import { hash } from '@ember/helper'`;

This should be built in, and not require an import.



An FCP is required before merging this PR to advance to Accepted.

Upon merging this PR, automation will open a draft PR for this RFC to move to the Ready for Released Stage.

Exploring Stage Description

This stage is entered when the Ember team believes the concept described in the RFC should be pursued, but the RFC may still need some more work, discussion, answers to open questions, and/or a champion before it can move to the next stage.

An RFC is moved into Exploring with consensus of the relevant teams. The relevant team expects to spend time helping to refine the proposal. The RFC remains a PR and will have an Exploring label applied.

An Exploring RFC that is successfully completed can move to Accepted with an FCP is required as in the existing process. It may also be moved to Closed with an FCP.

Accepted Stage Description

To move into the "accepted stage" the RFC must have complete prose and have successfully passed through an "FCP to Accept" period in which the community has weighed in and consensus has been achieved on the direction. The relevant teams believe that the proposal is well-specified and ready for implementation. The RFC has a champion within one of the relevant teams.

If there are unanswered questions, we have outlined them and expect that they will be answered before Ready for Release.

When the RFC is accepted, the PR will be merged, and automation will open a new PR to move the RFC to the Ready for Release stage. That PR should be used to track implementation progress and gain consensus to move to the next stage.

Checklist to move to Exploring

  • The team believes the concepts described in the RFC should be pursued.
  • The label S-Proposed is removed from the PR and the label S-Exploring is added.
  • The Ember team is willing to work on the proposal to get it to Accepted

Checklist to move to Accepted

  • This PR has had the Final Comment Period label has been added to start the FCP
  • The RFC is announced in #news-and-announcements in the Ember Discord.
  • The RFC has complete prose, is well-specified and ready for implementation.
    • All sections of the RFC are filled out.
    • Any unanswered questions are outlined and expected to be answered before Ready for Release.
    • "How we teach this?" is sufficiently filled out.
  • The RFC has a champion within one of the relevant teams.
  • The RFC has consensus after the FCP period.

@github-actions github-actions bot added the S-Proposed In the Proposed Stage label Dec 22, 2023
@NullVoxPopuli NullVoxPopuli changed the title Make hash built in Make hash built-in in strict-mode Dec 22, 2023
Copy link
Contributor

@MrChocolatine MrChocolatine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sceptical.

I feel like we went from "implicit things"
to "let's make them explicit for the greater good"
to now this "well actually let's make some things implicit again",
and possibly in a few months "you know what, actually let's make things explicit".

text/0999-make-hash-built-in.md Outdated Show resolved Hide resolved
text/0999-make-hash-built-in.md Outdated Show resolved Hide resolved
@NullVoxPopuli
Copy link
Contributor Author

I feel like we went from "implicit things"
to "let's make them explicit for the greater good"
to now this "well actually let's make some things implicit again",
and possibly in a few months "you know what, actually let's make things explicit".

The problem for arrays and hashes comes down to the fact that these do not exists as language primitives, and every other language makes arrays and objects super super easy.

fn is out of place for the reason stated in that rfc.

On is the only true convenience, imo. But that's the point of a framework, yea? To be convenient?

@lifeart
Copy link

lifeart commented Dec 24, 2023

Should we create real objects in hash?
Related issue: glimmerjs/glimmer-vm#1429

@NullVoxPopuli
Copy link
Contributor Author

Should we create real objects in hash?

would we not want them to crazy fine-grained reactivity for each property? the equiv of a TrackedObject?

@ef4 ef4 added the S-Exploring In the Exploring RFC Stage label Jan 19, 2024
@kategengler kategengler removed the S-Proposed In the Proposed Stage label Feb 9, 2024
@achambers
Copy link
Contributor

RFC Review (1) are in favour of this.

@josemarluedke
Copy link

For folks using gts/gjs, to keep importing the hash helper is just a constant pain not to have what you would have in JS by default.

I'm in favor of making hash and others built-in in strict mode. However, what I really wish we had was the syntax to just create an object in hbs scope. Something like <MyComponent @options={{ { option1: true, object2: false } }}. I know that 3 braces mean something else, but that would be the syntax I would be looking for.

Anyway, making hash build-in in strict mode would be a good step forward.

@kategengler
Copy link
Member

We decided to move this RFC along with #998, #997, #1000 into FCP to accepted today at the RFC meeting assuming it has the same details as #997

@flashios09
Copy link

Can we support/add obj or object as an alias for hash ?
The term hash has several meanings depending on the context, especially for non ember users, they will be confused:

<User @model={{hash password='ThR4ceKrnOK65Z6' }} />

Is this a hash function that will hash the password ?

While the term obj or object is more friendly/explicit:

<User @model={{obj password='ThR4ceKrnOK65Z6'}} />

Ok, this is just an object helper: {password: 'ThR4ceKrnOK65Z6'}

@kategengler
Copy link
Member

@flashios09 This is a reasonable point and one that has come up before. However, since hash has a long-standing history in Ember (aka we already have had that teaching problem for a long time) and we want this RFC to be the relatively minimal "make it built in", I would suggest opening a new RFC.

@NullVoxPopuli
Copy link
Contributor Author

NullVoxPopuli commented May 20, 2024

Additionally, I think everyone is in favor of object and array literals, rather than helpers -- which still need design, but would eliminate the problem altogether.

imagine (something like):

<template mode='exp'>
  <Foo 
    @someArray={ [1, 2, 3, 4] } 
    @someObj={ { a: 1, b: 2, c: 3 } }
  />
</template>

@flashios09
Copy link

@flashios09 This is a reasonable point and one that has come up before. However, since hash has a long-standing history in Ember (aka we already have had that teaching problem for a long time) and we want this RFC to be the relatively minimal "make it built in", I would suggest opening a new RFC.

As an alias, which means supporting the hash AND an alias like obj, why we would do the job TWICE ?

@kategengler
Copy link
Member

As an alias, which means supporting the hash AND an alias like obj, why we would do the job TWICE ?

Because adding an alias to obj is a different task and proposal entirely than building in the already existing hash helper.

@flashios09
Copy link

Because adding an alias to obj is a different task

A different task ?

if (helper === 'hash') {
  import('hash').from('@ember/helper');
}

So we will need a new RFC to just do this ?

if (helper === 'hash' || helper === 'obj') {
 import('hash').from('@ember/helper');
}

@NullVoxPopuli is it possible to update this RFC to support the obj alias ?

@NullVoxPopuli
Copy link
Contributor Author

@flashios09 , given that we're going gjs/gts for all things as default (soon 🤞 ), folks are free to do:

import { hash as obj } from '@ember/helper';
import Foo from './foo';

<template>
  <Foo @data={{obj a=1 b=2}} />
</template>

@achambers achambers enabled auto-merge June 12, 2024 14:18
@achambers achambers merged commit 70dcf1e into emberjs:master Jun 12, 2024
7 of 8 checks passed
@NullVoxPopuli NullVoxPopuli deleted the make-hash-built-in branch June 12, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Final Comment Period S-Exploring In the Exploring RFC Stage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants