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

Update to Ember 4.12.2 #1076

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .ember-cli
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,11 @@

Setting `disableAnalytics` to true will prevent any data from being sent.
*/
"disableAnalytics": false
"disableAnalytics": false,

/**
Setting `isTypeScriptProject` to true will force the blueprint generators to generate TypeScript
rather than JavaScript by default, when a TypeScript version of a given blueprint is available.
*/
"isTypeScriptProject": false
}
3 changes: 3 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,7 @@
# ember-try
/.node_modules.ember-try/
/bower.json.ember-try
/npm-shrinkwrap.json.ember-try
/package.json.ember-try
/package-lock.json.ember-try
/yarn.lock.ember-try
22 changes: 10 additions & 12 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@

module.exports = {
root: true,
parser: 'babel-eslint',
parser: '@babel/eslint-parser',
parserOptions: {
ecmaVersion: 2018,
ecmaVersion: 'latest',
sourceType: 'module',
ecmaFeatures: {
legacyDecorators: true,
requireConfigFile: false,
babelOptions: {
plugins: [
['@babel/plugin-proposal-decorators', { decoratorsBeforeExport: true }],
],
},
},
plugins: ['ember'],
Expand All @@ -30,6 +33,7 @@ module.exports = {
files: [
'./.eslintrc.js',
'./.prettierrc.js',
'./.stylelintrc.js',
'./.template-lintrc.js',
'./ember-cli-build.js',
'./testem.js',
Expand All @@ -45,16 +49,10 @@ module.exports = {
browser: false,
node: true,
},
plugins: ['node'],
extends: ['plugin:node/recommended'],
rules: {
// this can be removed once the following is fixed
// https://github.com/mysticatea/eslint-plugin-node/issues/77
'node/no-unpublished-require': 'off',
},
extends: ['plugin:n/recommended'],
},
{
// Test files:
// test files
files: ['tests/**/*-test.{js,ts}'],
extends: ['plugin:qunit/recommended'],
rules: {
Expand Down
26 changes: 16 additions & 10 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ on:
push:
branches:
- main
pull_request:
pull_request: {}

concurrency:
group: ci-${{ github.head_ref || github.ref }}
cancel-in-progress: true

env:
NODE_VERSION: 20
Expand All @@ -18,14 +22,16 @@ jobs:
timeout-minutes: 3
steps:
- name: Check out a copy of the repo
uses: actions/checkout@v2
uses: actions/checkout@v3

- uses: mansona/npm-lockfile-version@v1
with:
version: 3

- name: Use Node.js ${{ env.NODE_VERSION }}
uses: actions/setup-node@v2
uses: actions/setup-node@v3
with:
cache: 'npm'
cache: "npm"
node-version: ${{ env.NODE_VERSION }}

- name: Install dependencies
Expand All @@ -40,12 +46,12 @@ jobs:
timeout-minutes: 5
steps:
- name: Check out a copy of the repo
uses: actions/checkout@v2
uses: actions/checkout@v3

- name: Use Node.js ${{ env.NODE_VERSION }}
uses: actions/setup-node@v2
uses: actions/setup-node@v3
with:
cache: 'npm'
cache: "npm"
node-version: ${{ env.NODE_VERSION }}

- name: Install dependencies
Expand All @@ -64,14 +70,14 @@ jobs:
timeout-minutes: 5
steps:
- name: Check out a copy of the repo
uses: actions/checkout@v2
uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.sha }}

- name: Use Node.js ${{ env.NODE_VERSION }}
uses: actions/setup-node@v2
uses: actions/setup-node@v3
with:
cache: 'npm'
cache: "npm"
node-version: ${{ env.NODE_VERSION }}

- name: Install dependencies
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ jsconfig.json
# ember-try
/.node_modules.ember-try/
/bower.json.ember-try
/npm-shrinkwrap.json.ember-try
/package.json.ember-try

/.lighthouseci/
Expand Down
4 changes: 4 additions & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@
/coverage/
!.*
.eslintcache
.lint-todo/

# ember-try
/.node_modules.ember-try/
/bower.json.ember-try
/npm-shrinkwrap.json.ember-try
/package.json.ember-try
/package-lock.json.ember-try
/yarn.lock.ember-try
9 changes: 8 additions & 1 deletion .prettierrc.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
'use strict';

module.exports = {
singleQuote: true,
overrides: [
{
files: '*.{js,ts}',
options: {
singleQuote: true,
},
},
],
};
8 changes: 8 additions & 0 deletions .stylelintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# unconventional files
/blueprints/*/files/

# compiled output
/dist/

# addons
/.node_modules.ember-try/
6 changes: 0 additions & 6 deletions .stylelintrc

This file was deleted.

8 changes: 8 additions & 0 deletions .stylelintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use strict';

module.exports = {
extends: ['stylelint-config-standard', 'stylelint-prettier/recommended'],
rules: {
'selector-class-pattern': null,
},
};
5 changes: 5 additions & 0 deletions .template-lintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,9 @@

module.exports = {
extends: 'recommended',
rules: {
'no-curly-component-invocation': {
allow: ['format-date-time', 'html-safe', 'printf'],
},
},
};
40 changes: 20 additions & 20 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,52 +10,52 @@ Components come from [ember-styleguide](https://github.com/ember-learn/ember-sty

You will need the following things properly installed on your computer.

* [Git](https://git-scm.com/)
* [Node.js](https://nodejs.org/)
* [Ember CLI](https://ember-cli.com/)
* [Google Chrome](https://google.com/chrome/)
- [Git](https://git-scm.com/)
- [Node.js](https://nodejs.org/) (with npm)
- [Ember CLI](https://cli.emberjs.com/release/)
- [Google Chrome](https://google.com/chrome/)

## Installation

* `git clone <repository-url>` this repository
* `cd ember-website`
* `npm install`
- `git clone <repository-url>` this repository
- `cd ember-website`
- `npm install`

## Running / Development

* `npm start`
* Visit your app at [http://localhost:4200](http://localhost:4200).
* Visit your tests at [http://localhost:4200/tests](http://localhost:4200/tests).
- `npm start`
- Visit your app at [http://localhost:4200](http://localhost:4200).
- Visit your tests at [http://localhost:4200/tests](http://localhost:4200/tests).

### Code Generators

Make use of the many generators for code, try `ember help generate` for more details

### Running Tests

* `npm test`
* `npm test --server`
- `npm test`
- `npm test --server`

### Linting

* `npm run lint`
* `npm run lint:fix`
- `npm run lint`
- `npm run lint:fix`

### Building

* `npm run build` (production)
- `npm run build` (production)

### Deploying

The app is continuously deployed to Netlify when a pull request is merged and passes continuous integration.

## Further Reading / Useful Links

* [ember.js](https://emberjs.com/)
* [ember-cli](https://ember-cli.com/)
* Development Browser Extensions
* [ember inspector for chrome](https://chrome.google.com/webstore/detail/ember-inspector/bmdblncegkenkacieihfhpjfppoconhi)
* [ember inspector for firefox](https://addons.mozilla.org/en-US/firefox/addon/ember-inspector/)
- [ember.js](https://emberjs.com/)
- [ember-cli](https://cli.emberjs.com/release/)
- Development Browser Extensions
- [ember inspector for chrome](https://chrome.google.com/webstore/detail/ember-inspector/bmdblncegkenkacieihfhpjfppoconhi)
- [ember inspector for firefox](https://addons.mozilla.org/en-US/firefox/addon/ember-inspector/)

## Sponsors

Expand Down
4 changes: 1 addition & 3 deletions app/components/cta-emberconf.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@
target="_blank"
>
<img
alt=""
role="presentation"
src="/images/brand/virtual-emberconf-2022-logo.svg"
alt="" src="/images/brand/virtual-emberconf-2022-logo.svg"
>
</a>
</div>
Expand Down
1 change: 1 addition & 0 deletions app/components/ember-community-survey/introduction.hbs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<div class="bg-dark bg-shape-boxes">
<div class="container">
{{!-- template-lint-disable no-empty-headings --}}
<h1>
<img
alt={{@surveyLogoAlt}}
Expand Down
2 changes: 1 addition & 1 deletion app/components/index/ember-addons.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
class="addon-tabs--header"
role="tablist"
>
{{!-- template-lint-disable "no-down-event-binding" --}}
{{!-- template-lint-disable "no-pointer-down-event-binding" --}}
{{#each this.emberAddons as |emberAddon index|}}
<Index::EmberAddons::Tab
@currentTabId={{this.currentTabId}}
Expand Down
7 changes: 7 additions & 0 deletions app/routes/community/meetups/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,11 @@ export default class CommunityMeetupsIndexRoute extends Route {
model() {
return this.store.findAll('meetup');
}

afterModel() {
// ember-data runs flushAllPendingFetches using setTimeout, so outside of the Ember runloop.
// Under prember this causes `Attempted to call store.adapterFor(), but the store instance has already been destroyed.`
// So wait a bit
return new Promise((resolve) => setTimeout(resolve, 10));
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an ugly workaround for an issue with ember-data and FastBoot/prember. e-d calls flushAllPendingFetches with setTimeout (which Ember's settledness does not capture, I believe that's the problem), but as the app is torn down immediately after rendering in prember, we are getting:

Assertion Failed: Attempted to call store.adapterFor(), but the store instance has already been destroyed.

I was not really willing to spend much time investigating a "proper" solution, as

  1. the issue might have been resolved in new releases maybe
  2. I think we should actually get rid of e-d here. I'll raise a separate issue to explain my thoughts...

Copy link
Member

Choose a reason for hiding this comment

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

One concern I have is that with the current context in the code comment is that it makes it quite daunting to ever touch this part of the code. Is it possible to link to a bug ticket. (Is this even considered a bug, it seems intentional?) Or state when/how this workaround could be removed.

Removing ED would solve the problem as well, but is out of scope for this PR.

Another alternative could be downgrading Ember Data to 4.11 I assume, if this "issue" got introduced in 4.12?

Copy link
Contributor Author

@simonihmig simonihmig Mar 3, 2024

Choose a reason for hiding this comment

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

I didn't file an issue, because I wouldn't want to do so for a version that is out of date and without knowing that the problem is still present in the most recent one. But also ED has deprecated FastBoot support, so likely that could mean they wouldn't even consider it a bug?

Tbh, I don't really remember what happened when downgrading to ED to 4.11, but I am pretty sure I tried. This work is already two months old, which hits my brain's capacity limit 😅

The path I would be pursuing is to remove ED here, so the code you are concerned with wouldn't be around for too long!

Copy link
Member

Choose a reason for hiding this comment

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

I'm also a bit concerned about this hack being merged 🤔 I accept your intention to potentially remove ember-data since it's such a simple request response mode, but the intention was for the website to be a good simple demonstration of the capabilities in the default Ember blueprint. I think we should dig a bit deeper into this issue and potentially draft some help from the Ember data team especially if this issue is potentially affecting other people relying on ember-data and prember

}
7 changes: 7 additions & 0 deletions app/routes/ember-users.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,11 @@ export default class EmberUsersRoute extends Route {
model() {
return this.store.findAll('user');
}

afterModel() {
// ember-data runs flushAllPendingFetches using setTimeout, so outside of the Ember runloop.
// Under prember this causes `Attempted to call store.adapterFor(), but the store instance has already been destroyed.`
// So wait a bit
return new Promise((resolve) => setTimeout(resolve, 10));
}
}
7 changes: 7 additions & 0 deletions app/routes/learn.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,11 @@ export default class LearnRoute extends Route {
model() {
return this.store.findAll('showcase');
}

afterModel() {
// ember-data runs flushAllPendingFetches using setTimeout, so outside of the Ember runloop.
// Under prember this causes `Attempted to call store.adapterFor(), but the store instance has already been destroyed.`
// So wait a bit
return new Promise((resolve) => setTimeout(resolve, 10));
}
}
7 changes: 7 additions & 0 deletions app/routes/mascots.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,11 @@ export default class MascotsRoute extends Route {
model() {
return this.store.findAll('tomster');
}

afterModel() {
// ember-data runs flushAllPendingFetches using setTimeout, so outside of the Ember runloop.
// Under prember this causes `Attempted to call store.adapterFor(), but the store instance has already been destroyed.`
// So wait a bit
return new Promise((resolve) => setTimeout(resolve, 10));
}
}
7 changes: 7 additions & 0 deletions app/routes/releases.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,11 @@ export default class ReleasesRoute extends Route {
model() {
return this.store.findAll('project');
}

afterModel() {
// ember-data runs flushAllPendingFetches using setTimeout, so outside of the Ember runloop.
// Under prember this causes `Attempted to call store.adapterFor(), but the store instance has already been destroyed.`
// So wait a bit
return new Promise((resolve) => setTimeout(resolve, 10));
}
}
11 changes: 9 additions & 2 deletions app/routes/releases/beta.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,15 @@ export default class ReleasesBetaRoute extends Route {

model() {
return hash({
ember: this.store.find('project', 'ember/beta'),
emberData: this.store.find('project', 'emberData/beta'),
ember: this.store.findRecord('project', 'ember/beta'),
emberData: this.store.findRecord('project', 'emberData/beta'),
});
}

afterModel() {
// ember-data runs flushAllPendingFetches using setTimeout, so outside of the Ember runloop.
// Under prember this causes `Attempted to call store.adapterFor(), but the store instance has already been destroyed.`
// So wait a bit
return new Promise((resolve) => setTimeout(resolve, 10));
}
}
Loading