-
Notifications
You must be signed in to change notification settings - Fork 31
chore: 🤖 create patched ember-loading to use modern ember-concurrency #3015
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
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
00befb0
to
9a8b485
Compare
9a8b485
to
f9e26ae
Compare
@@ -0,0 +1,112 @@ | |||
diff --git a/addon/services/loading.ts b/addon/services/loading.ts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
module.exports = { | ||
name: require('./package').name, | ||
+ options: { | ||
+ babel: { | ||
+ plugins: [ | ||
+ require.resolve('ember-concurrency/async-arrow-task-transform'), | ||
+ ], | ||
+ }, | ||
+ }, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f9e26ae
to
1a74a79
Compare
"ember-try-config>package-json": "^10.0.1", | ||
"ember-stargate@^0.6.0>@glimmer/component": "^2.0.0" | ||
"ember-stargate@^0.6.0>@glimmer/component": "^2.0.0", | ||
"ember-loading>ember-concurrency": "^4.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed? It looks like you changed the ember-loading
package.json to instead make ember-concurrency
a peer dep, would it not just load our app's version now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Patching the dependency's package.json doesn't actually change what gets installed so the manual overrides to ember-concurrency v4 is also needed.
I treated the patch as what the package output should be, but unfortunately the way the patches are resolved don't impact how the dependencies are managed and stored in the lockfile. Aside from changing the version I wanted to hint at that ember-concurrency-async
and ember-concurrency-ts
shouldn't be used, but those are also in the lockfile still. If this is more confusing than helpful I can remove the dependency changes from the package.json
patch
1a74a79
to
06a62e0
Compare
Description
This adds a patch for
ember-loading
dependency since it uses older ember-concurrency versions and supporting addons which prevent us from using vite-compatible addons. Patching the dependency'spackage.json
doesn't actually change what gets installed so the manual overrides toember-concurrency
v4 is also needed.How to Test
is-loading: {{is-loading}}
to theapplication.hbs
in desktop or adminserver.timing = 1000
to the mirage scenario or use the app with a real apiis-loading: true
in the browser and then when the loading is finished theis-loading: false
should be shownChecklist
a11y-tests
label to run a11y audit tests if neededPCI review checklist
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.