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 deprecations failing the build #1448

Merged
merged 2 commits into from
Mar 14, 2021
Merged

Fix deprecations failing the build #1448

merged 2 commits into from
Mar 14, 2021

Conversation

simonihmig
Copy link
Contributor

  • ignore upstream deprecations we cannot influence here (modifier-manager)
  • don't setup deprecation tracking for tests that test positional params in our <LinkTo> subclasses, as we have those tests for compatibility with that legacy API (positional params), but can regard that as deprecated from our side as well

@@ -64,32 +60,30 @@ module('Integration | Component | bs-dropdown/menu/link-to', function (hooks) {
});
});

if (supportsAngleBracketsLinkTo) {
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 checking hasEmberVersion(3, 10), i.e. guards against Ember <3.10, which we don't support anymore.

test('simple route link', async function (assert) {
await render(hbs`<BsDropdown::menu::link-to @route="index">Link</BsDropdown::menu::link-to>`);
module('<LinkTo> properties', function (hooks) {
setupNoDeprecations(hooks);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The diff looks large, but basically I just moved that setupNoDeprecations call from the outer module() to this submodule, thus not applying it to the module testing positional params.

@simonihmig
Copy link
Contributor Author

Hm, I tested this locally with beta (which is passing now), but apparently canary has a new deprecation added. Will tackle this later...

@lupestro
Copy link

While you're at it, do you want to do all the {{#if hasBlock}} to {{#if has-block}} adjustments, with hasBlock also deprecated in beta? (RFC #689). I have seen these deprecations show up in building one or two of my addons, so I did a grep of the whole GIT forest for all my projects, including the node_modules, found the following from ember-bootstrap, and confirmed they're still present in master.

addon\templates\components\bs3\bs-form\element\control\radio.hbs
addon\templates\components\bs3\bs-form\element\control\radio.hbs
addon\templates\components\bs3\bs-form\element\label.hbs
addon\templates\components\bs3\bs-modal\header.hbs
addon\templates\components\bs3\bs-modal\header.hbs
addon\templates\components\bs3\bs-navbar\toggle.hbs
addon\templates\components\bs4\bs-form\element\control\radio.hbs
addon\templates\components\bs4\bs-form\element\label.hbs
addon\templates\components\bs4\bs-modal\header.hbs
addon\templates\components\bs4\bs-modal\header.hbs
addon\templates\components\bs4\bs-navbar\toggle.hbs
addon\templates\components\common\bs-accordion\item.hbs
addon\templates\components\common\bs-form\element.hbs
addon\templates\components\common\bs-modal\footer.hbs
addon\templates\components\common\bs-progress\bar.hbs
addon\templates\components\common\bs-progress\bar.hbs
addon\templates\components\common\bs-tooltip.hbs

I'd open a PR myself, pull a fork, etc., but, as you've already got a PR open adjusting for the recent raft of fresh deprecations, hopefully these will be easy. :)

Now if I could only figure out which upstream module was dropping the modifier-manager version stuff - so far I've got about 8 candidates to look at.

@simonihmig
Copy link
Contributor Author

While you're at it, do you want to do all the {{#if hasBlock}} to {{#if has-block}} adjustments, with hasBlock also deprecated in beta?

This should have been addressed already be #1421! It's just not released yet.

@lupestro
Copy link

This should have been addressed already be #1421! It's just not released yet.

I just checked - #1421 is merged and didn't try to address this particular deprecation. hasBlock rather than has-block is present in handlebars in master, so no fix has been merged for it, and I didn't see another open PR to address them.

@simonihmig
Copy link
Contributor Author

Oh, you are right. The PR did tackle a few, but apparently not all.

I do however wonder why you see deprecations for this? As any deprecations will fail CI, and beta is now passing again with this PR!

@simonihmig
Copy link
Contributor Author

The remaining deprecation in Canary here comes from extending LinkComponent, which will be tackled by a separate PR. So going to merge this.

@simonihmig simonihmig merged commit 6aeead2 into master Mar 14, 2021
@simonihmig simonihmig deleted the deprecations branch March 14, 2021 23:30
@simonihmig
Copy link
Contributor Author

I'd open a PR myself

@lupestro If you have the time, this would be very welcome! 🙂

@lupestro
Copy link

I'll put it on my list for some evening this week. I'll do due diligence, but my contribution will want an attentive reviewer, as I build projects for my team that use ember-bootstrap but I haven't used it in my own work yet.

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