Skip to content

Conversation

@ijlee2
Copy link
Contributor

@ijlee2 ijlee2 commented May 27, 2025

Background

We use and maintain a fork of ember-table at work, and noticed that the pod structure is still used for components. Generally speaking, ember-source@6.0.0 removed pod support for components, so ember-table may end up becoming a blocker to developers who want to update Ember to v6.

I suspect that the ember-release scenario (i.e. 6.4.0) is currently passing, because the classic components make use of the layout key. Removing the pod structure now would help maintainers and contributors glimmerize these components (Glimmer + pod doesn't work) and introduce the v2 addon format.

What changed?

In this pull request, I updated eslint and test dependencies so that we can keep this project up-to-date. The actual unpodding is done in #1173.

@ijlee2 ijlee2 mentioned this pull request May 27, 2025
@ijlee2 ijlee2 marked this pull request as ready for review May 27, 2025 06:44
module.exports = {
extends: ['@addepar', '@addepar/eslint-config/ember'],
parser: 'babel-eslint',
root: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on ember-cli@5.12 blueprints. I didn't introduce eslint@v9 and flat config to limit scope.

run: yarn install --frozen-lockfile

- name: Run Tests
run: node_modules/.bin/ember try:one ${{ matrix.ember-version }} --skip-cleanup
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a good practice to refer to node_modules/.bin/ember (the correct path may depend on which package manager is used).

Comment on lines -20 to -24
# ember-try
.node_modules.ember-try/
bower.json.ember-try
package.json.ember-try
yarn.lock.ember-try
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not needed in ember-try@v4.

yarn.lock Outdated
@@ -53,12 +53,21 @@
js-tokens "^4.0.0"
picocolors "^1.0.0"

"@babel/code-frame@^7.27.1":
Copy link
Contributor Author

@ijlee2 ijlee2 May 27, 2025

Choose a reason for hiding this comment

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

I only ran yarn install. I normally run yarn-deuplicate in my projects afterwards, but didn't here to be on the safe side.

@ijlee2 ijlee2 changed the title Updated dependencies Updated eslint and test dependencies May 27, 2025
@ijlee2 ijlee2 force-pushed the update-dependencies branch from c9da16a to 5698750 Compare May 27, 2025 07:06
@mixonic mixonic force-pushed the update-dependencies branch from 66d175e to dc3ca9a Compare July 2, 2025 16:23
Copy link
Member

@mixonic mixonic left a comment

Choose a reason for hiding this comment

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

Approving to allow tests to advance

Comment on lines -9 to -14
ENV['ember-a11y-testing'] = {
componentOptions: {
turnAuditOff: true, // Change to true to disable the audit in development
visualNoiseLevel: 2,
axeViolationClassNames: ['alert-box', 'alert-box--a11y'],
},
Copy link
Member

Choose a reason for hiding this comment

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

Where does this configuration migrate to?

Copy link
Contributor Author

@ijlee2 ijlee2 Jul 9, 2025

Choose a reason for hiding this comment

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

The configuration was meant for the dummy app, so I'd guess tests/dummy/config/environment.js. You'll want to check if ember-a11y-testing still supports these options.

https://github.com/ember-a11y/ember-a11y-testing/tree/v7.1.2-ember-a11y-testing

@mixonic
Copy link
Member

mixonic commented Jul 2, 2025

Seems like Ember 3.12 tests fail reliably here- But good news, we can just drop Ember 3.12 from the CI suite on master per #995. Just needs a PR to do it. I can follow through on that and rebase this again (I rebased it once to address a merge conflict)

@mixonic mixonic force-pushed the update-dependencies branch from dc3ca9a to 3fb4bbb Compare July 3, 2025 15:43
@mixonic
Copy link
Member

mixonic commented Jul 3, 2025

I've force pushed to rebase atop master. This should remove Ember < 3.28 from the CI system.

@mixonic mixonic merged commit 2f0a7ba into Addepar:master Jul 3, 2025
17 of 18 checks passed
@mixonic mixonic added the 6.0 label Jul 3, 2025
@mixonic
Copy link
Member

mixonic commented Jul 3, 2025

Included in 6.0.0-9

@ijlee2 ijlee2 deleted the update-dependencies branch July 9, 2025 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants