Skip to content

Commit

Permalink
Merge branch 'master' into advance-rfc-0821
Browse files Browse the repository at this point in the history
  • Loading branch information
wagenet authored Feb 17, 2023
2 parents 6e71470 + 5e302a8 commit f10ed2c
Show file tree
Hide file tree
Showing 11 changed files with 123 additions and 27 deletions.
2 changes: 1 addition & 1 deletion .github/actions/setup-rfcs-tooling/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ runs:
with:
repository: emberjs/rfcs-tooling
path: rfcs-tooling
ref: 'v2.1.1'
ref: 'v3.0.0'

- uses: actions/setup-node@v3
with:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/advance-rfc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ jobs:
echo "A new RFC was added"
echo "value=true" >> $GITHUB_OUTPUT
else
node rfcs-tooling/has-stage-changed.js ${{ github.event.before }} ${{ steps.rfcs.outputs.modified-rfc }}
node rfcs-tooling/scripts/has-stage-changed.mjs ${{ github.event.before }} ${{ steps.rfcs.outputs.modified-rfc }}
if [[ $? == 0 ]]; then
echo "## Yes, stage has changed" >> $GITHUB_STEP_SUMMARY
echo "value=true" >> $GITHUB_OUTPUT
Expand All @@ -69,7 +69,7 @@ jobs:
if: steps.has-stage-changed.outputs.value == 'true'
id: new-stage
run: |
new_stage=`node rfcs-tooling/find-next-stage.js ${{ steps.rfcs.outputs.modified-rfc }}`
new_stage=`node rfcs-tooling/scripts/find-next-stage.mjs ${{ steps.rfcs.outputs.modified-rfc }}`
echo "New Stage: $new_stage"
echo "value=$new_stage" >> $GITHUB_OUTPUT
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ jobs:
uses: ./.github/actions/setup-rfcs-tooling

- name: Lint the frontmatter of all RFCs
run: node ./rfcs-tooling/lint-rfc-frontmatter.js text/*.md
run: node ./rfcs-tooling/scripts/lint-rfc-frontmatter.mjs text/*.md
2 changes: 1 addition & 1 deletion .github/workflows/generate-rfc-frontmatter-json.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:

- name: Generate frontmatter data file
run: |
node rfcs-tooling/list-frontmatter.js text/*.md > rfc-data.json
node rfcs-tooling/scripts/list-frontmatter.mjs text/*.md > rfc-data.json
- uses: actions/upload-artifact@v3
with:
Expand Down
7 changes: 3 additions & 4 deletions .github/workflows/newly-added-rfcs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ jobs:

- name: Verify stage of newly added RFC is `accepted` in frontmatter
run: |
frontmatter=`node rfcs-tooling/rfc-frontmatter.js ${{ needs.check-rfcs.outputs.modified-rfc }}`
frontmatter=`node rfcs-tooling/scripts/rfc-frontmatter.mjs ${{ needs.check-rfcs.outputs.modified-rfc }}`
stage=`echo $frontmatter | jq '.stage'`
if [[ $stage != '"accepted"' ]]; then
echo "::error::Newly added RFCs must have the stage 'accepted' in the frontmatter"
Expand All @@ -113,8 +113,7 @@ jobs:
- name: Test RFC Filename matches PR Number that adds it
env:
PR_NUMBER: ${{ github.event.pull_request.number }}
run: node check-filename-matches-pr.js $PR_NUMBER ${{ needs.check-rfcs.outputs.modified-rfc }}
working-directory: rfcs-tooling
run: node rfcs-tooling/scripts/check-filename-matches-pr.mjs $PR_NUMBER ${{ needs.check-rfcs.outputs.modified-rfc }}

check-accepted-pr-url:
name: Verify Accepted PR URL is correct
Expand All @@ -129,7 +128,7 @@ jobs:

- name: Verify Accepted PR URL is correct
run: |
frontmatter=`node rfcs-tooling/rfc-frontmatter.js ${{ needs.check-rfcs.outputs.modified-rfc }}`
frontmatter=`node rfcs-tooling/scripts/rfc-frontmatter.mjs ${{ needs.check-rfcs.outputs.modified-rfc }}`
accepted_pr=`echo $frontmatter | jq '.prs.accepted'`
accepted_pr=${accepted_pr//\"/}
expected_pr="${{ github.event.pull_request.html_url }}"
Expand Down
12 changes: 7 additions & 5 deletions .github/workflows/open-advancement-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,17 @@ jobs:

- name: Update frontmatter
run: |
node rfcs-tooling/update-rfc-stage.js ${{ inputs.new-stage }} ${{ inputs.rfc-path }}
node rfcs-tooling/scripts/update-rfc-stage.mjs ${{ inputs.new-stage }} ${{ inputs.rfc-path }}
- name: Set variables for use in PR
id: pr-variables
run: |
frontmatter=`node rfcs-tooling/rfc-frontmatter.js ${{ inputs.rfc-path }}`
frontmatter=`node rfcs-tooling/scripts/rfc-frontmatter.mjs ${{ inputs.rfc-path }}`
ready_for_release_pr=`echo $frontmatter | jq '.prs."ready-for-release"'`
ready_for_release_pr=${ready_for_release_pr//\"/}
released_pr=`echo $frontmatter | jq '.prs.released'`
released_pr=${released_pr//\"/}
title=`echo $frontmatter | jq '.title'`
if [[ ${{ inputs.new-stage }} == "ready-for-release" ]]; then
pretty_stage="Ready for Release"
template=`sed -e 's/__RFC_NUMBER__/${{ inputs.rfc-number }}/g' .github/PULL_REQUEST_TEMPLATE/advance-to-ready-for-release.md`
Expand All @@ -53,23 +54,24 @@ jobs:
echo 'EOF' >> $GITHUB_OUTPUT
echo "Pretty Stage: $pretty_stage"
echo "pretty-stage=$pretty_stage" >> $GITHUB_OUTPUT
echo "title=$title" >> $GITHUB_OUTPUT
- name: Open PR
id: create-pr
uses: peter-evans/create-pull-request@v4.2.0
with:
token: ${{ secrets.personal-access-token }}
commit-message: "Advance RFC to Stage ${{ inputs.new-stage }}"
commit-message: "Advance RFC {{ inputs.rfc-number }} to Stage ${{ inputs.new-stage }}"
add-paths: 'text'
branch: "advance-rfc-${{ inputs.rfc-number }}"
title: "Advance RFC #${{ inputs.rfc-number}} to Stage ${{ steps.pr-variables.outputs.pretty-stage }}"
title: "Advance RFC #${{ inputs.rfc-number}} `${{ steps.pr-variables.outputs.title }}` to Stage ${{ steps.pr-variables.outputs.pretty-stage }}"
body: "${{ steps.pr-variables.outputs.body }}"
labels: "RFC Advancement,S-${{ steps.pr-variables.outputs.pretty-stage}}"
draft: true

- name: Add new PR link to RFC frontmatter
run: |
node rfcs-tooling/update-advancement-pr.js ${{ inputs.rfc-path }} ${{ inputs.new-stage}} ${{ steps.create-pr.outputs.pull-request-url }}
node rfcs-tooling/scripts/update-advancement-pr.mjs ${{ inputs.rfc-path }} ${{ inputs.new-stage}} ${{ steps.create-pr.outputs.pull-request-url }}
- name: Update PR
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/trigger-opening-advancement-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
- name: Find new stage
id: new-stage
run: |
new_stage=`node rfcs-tooling/find-next-stage.js ${{ inputs.rfc-path }}`
new_stage=`node rfcs-tooling/scripts/find-next-stage.mjs ${{ inputs.rfc-path }}`
echo "New Stage: $new_stage"
echo "value=$new_stage" >> $GITHUB_OUTPUT
Expand Down
12 changes: 8 additions & 4 deletions text/0236-deprecation-ember-string.md
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
---
stage: accepted
stage: released
start-date: 2017-07-14T00:00:00.000Z
release-date: 2020-12-28T00:00:00.000Z
release-date: 2023-01-12T00:00:00.000Z
release-versions:
ember-source: v4.10.0
teams:
- framework
- typescript
prs:
accepted: https://github.com/emberjs/rfcs/pull/236
accepted: 'https://github.com/emberjs/rfcs/pull/236'
ready-for-release: 'https://github.com/emberjs/rfcs/pull/892'
released: 'https://github.com/emberjs/rfcs/pull/897'
project-link:
meta:
tracking: https://github.com/emberjs/rfc-tracking/issues/26
tracking: 'https://github.com/emberjs/ember.js/issues/20340'
legacy-tracking: 'https://github.com/emberjs/rfc-tracking/issues/26'
---

# Summary
Expand Down
6 changes: 3 additions & 3 deletions text/0756-helper-default-manager.md
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
---
stage: released # FIXME: This may be recommended
stage: recommended
start-date: 2021-05-17T00:00:00.000Z
release-date: 2022-05-13T00:00:00.000Z
release-versions:
ember-source: v4.5.0

teams:
- framework
- learning
prs:
accepted: https://github.com/emberjs/rfcs/pull/756
accepted: 'https://github.com/emberjs/rfcs/pull/756'
recommended: 'https://github.com/emberjs/rfcs/pull/866'
project-link:
---

Expand Down
89 changes: 89 additions & 0 deletions text/0774-implicit-record-route-loading.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
---
stage: accepted
start-date: 2021-11-14
release-date:
release-versions:
teams: # delete teams that aren't relevant
- framework
prs:
accepted: https://github.com/emberjs/rfcs/pull/774
---

# Deprecate Implicit Record Loading in Routes

## Summary

This RFC seeks to deprecate and remove the default record loading behaviour on Ember's Route. By consequence, this will also deprecate and remove the default store on every Route. This behaviour is likely something you either know you have or do not know but it may be occuring in your app.

```js
export default class PostRoute extends Route {
beforeModel() {
// do stuff
}

afterModel() {
// do stuff
}
}
```

In this example, the `model` hook is not defined. However, Ember will attempt to try a few things before rendering this route's template.

1. If there is a `store` property on your route, it will attempt to call it's `find` method. Assuming you have `ember-data` installed, you may be expecting this. The arguments will be extracted from the params.
a. For example, if a dynamic segment is `:post_id`, there exists logic to split on the underscore and find a record of type `post`.

2. As a fallback, it will attempt to define a `find` method and use your `Model` instance's find method to fetch. If a `Model` cannot be found or if the found `Model` does not have a `find` method, an assertion is thrown.

## Motivation

If a user does not define a `model` hook, a side effect of going to the network has long confused developers. Users have come to associate `Route.model()` with a hook that returns `ember-data` models in the absense of explicit injection. While this can be true, it is not wholly true. New patterns of data loading are becoming accepted in the community including opting to fetch data in a Component or using different libraries.

By removing this behaviour, we will encourage developers to explicitly define what data is being fetched and from where.

## Detailed design

We will issue a deprecation to [`findModel`](https://github.com/emberjs/ember.js/blob/017b11e2f58880869a5b8c647bf7f3199fc07f26/packages/%40ember/-internals/routing/lib/system/route.ts#L1376) notifying the user that if they want to continue this behaviour of attempting to fetch a resource implicitly, they should try and replicate with their own explicitly defined `model` hook.

In addition, we will include an [optional feature](https://github.com/emberjs/ember-optional-features) to disable this feature and clear the deprecation.

In v6.0.0, we will remove `findModel` and logic to determine arguments for this method. This will not remove returning the `transition` context when no `model` hook is defined.

## How we teach this

Most of this behaviour is lightly documented. Developers often come to this by mistake after some difficult searching. First, we will have to remove this sentence from the [docs](https://guides.emberjs.com/release/routing/defining-your-routes/#toc_dynamic-segments).

> The first reason is that Routes know how to fetch the right model by default, if you follow the convention.
A direct one to one replacement might look like this.

```js
import { inject as service } from '@ember/service';

export default class PostRoute extends Route {
// assuming you have ember-data installed
@service store;

beforeModel() {
// do stuff
}

model({ post_id }) {
return this.store.findRecord('post', post_id);
}

afterModel() {
// do stuff
}
}
```

## Alternatives

- Continue to provide fallback fetching behaviour but ensure no `assert` is called for users that have neither a store nor a `Model` with a `find` method.

## Open Questions

## Related links and RFCs
- [Deprecate defaultStore located at `Route.store`](https://github.com/emberjs/rfcs/issues/377)
- [Pre-RFC: Deprecate implicit injections (owner.inject)](https://github.com/emberjs/rfcs/issues/508)
- [Deprecate implicit record loading in routes](https://github.com/emberjs/rfcs/issues/557)
12 changes: 7 additions & 5 deletions text/0889-deprecate-ember-error.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
---
stage: accepted
start-date: 2022-12-15
release-date:
stage: released
start-date: 2022-12-15T00:00:00.000Z
release-date: 2023-01-12T00:00:00.000Z
release-versions:
teams: # delete teams that aren't relevant
ember-source: v4.10.0
teams:
- framework
- typescript
prs:
accepted: https://github.com/emberjs/rfcs/pull/889
accepted: 'https://github.com/emberjs/rfcs/pull/889'
released: 'https://github.com/emberjs/rfcs/pull/895'
project-link:
---

Expand Down

0 comments on commit f10ed2c

Please sign in to comment.