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

Fix compatibility with ember-render-helpers v0.2 #2157

Merged
merged 2 commits into from
Nov 11, 2024

Conversation

jelhan
Copy link
Contributor

@jelhan jelhan commented Oct 16, 2024

#2156 should have restored compatibility with ember-render-helpers@^0.2.0 but it hasn't. The test had a bug and therefore leading to a false sense of security. The helpers were renamed in the v1.0 release and the new names weren't available in v0.2.0.

This PR requires ember-render-helpers v0.2.1 which backports the new helper names.

It removes the test as it was not working as expected anyways. See #2157 (comment) and ember-cli/ember-try#970 for context.

@jelhan
Copy link
Contributor Author

jelhan commented Oct 16, 2024

I'm not sure if we can get that compatibility test reliable. Overrides (and resolutions) are only supported at the root of the project. But as far as I know Ember Try does not support modifying the package.json at the root of the project in a monorepo.

With the current setup it is coincidence if the ember-render-helpers version from the test-app or from ember-bootstrap package gets pulled into the build.

@jelhan jelhan force-pushed the fix-compatibility-with-ember-render-helpers-v0.2 branch from 5b8c767 to 7abc196 Compare November 11, 2024 09:51
@jelhan jelhan marked this pull request as ready for review November 11, 2024 09:54
@jelhan jelhan changed the title [DRAFT] Fix compatibility with ember-render-helpers v0.2 Fix compatibility with ember-render-helpers v0.2 Nov 11, 2024
@jelhan jelhan added the bug label Nov 11, 2024
@jelhan jelhan requested a review from SanderKnauff November 11, 2024 10:16
Copy link
Contributor

@SanderKnauff SanderKnauff left a comment

Choose a reason for hiding this comment

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

Looks good! Should these helpers be refactored away in the future?

@jelhan
Copy link
Contributor Author

jelhan commented Nov 11, 2024

Looks good! Should these helpers be refactored away in the future?

I think so. Conceptually they are the same as @ember/render-modifiers.

I haven't looked into why we are using them to be honest.

@jelhan jelhan merged commit 51e4700 into master Nov 11, 2024
22 checks passed
@jelhan jelhan deleted the fix-compatibility-with-ember-render-helpers-v0.2 branch November 11, 2024 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants