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 ember-maybe-in-element to 2.0.1 #406

Closed

Conversation

andreyfel
Copy link

@andreyfel andreyfel commented Aug 13, 2020

ember-maybe-in-element needs to be updated to ^2.0.1, so, it can use latest {{in-element}} API from Ember 3.20 and don't throw deprecation about using deprecated private {{-in-element}} API.
Update other dependencies as well
Update Ember to 3.20
Drop support of Ember<3.13 since 3.13 is a minimum supported version for ember-maybe-in-element@2.0.1

# Conflicts:
#	.eslintrc.js
#	.travis.yml
#	CONTRIBUTING.md
#	README.md
#	config/ember-try.js
ember-maybe-in-element needs to be updated to ^2.0.1, so, it can use
latest {{in-element}} API from Ember 3.20 and don't throw deprecation
about using deprecated private {{-in-element}} API.

Update other dependencies as well.
Ember 3.13 is a minimum supported version of
ember-maybe-in-element@2.0.1.

Fix eslint errors and use this.foo instead of this.get('foo').
Since this syntax is supported since Ember 3.1 this change is
compatible with current supported versions >= 3.13.
@andreyfel andreyfel force-pushed the update-ember-maybe-in-element branch from 3dd9042 to 8ba41ed Compare August 13, 2020 11:26
@andreyfel
Copy link
Author

@sir-dunxalot please take a look. This change probably needs to be released as next major version since it drops Node 8 and older Ember versions.

@maxfierke
Copy link
Collaborator

@andreyfel Dropping support for Ember < 3.13 would be a breaking change that would require bumping the major version, however I don't think we should do that just yet. I will likely be looking at relaxing the version constraint for ember-maybe-in-element to allow >= 0.4.0, < 3

@andreyfel
Copy link
Author

@maxfierke Does it worth the effort? These new versions are needed for users facing depreciation warnings in Ember 3.20. If someone is using old version of Ember he can keep using old version of ember-tooltips.

@maxfierke
Copy link
Collaborator

@maxfierke Does it worth the effort?

It doesn't require a high amount of effort to maintain support for these earlier versions and there are still many people using Ember < 3.13. We can support both < 3.13 and 3.20 without deprecations

@maxfierke
Copy link
Collaborator

Closing since merging #403 removes the deprecation warning on Ember 3.20 while retaining compatibility with earlier versions of Ember. Thank you, though!

@maxfierke maxfierke closed this Aug 14, 2020
@andreyfel
Copy link
Author

@maxfierke I see that #403 updates ember-maybe-in-element to ^1.0.0. The problem is that ember-basic-dropdown bumped its dependency from ember-maybe-in-element to ^2.0.0, see cibernox/ember-basic-dropdown#573. So, it makes it impossible to use the latest version of ember-basic-dropdown and ember-tooltips in a single app.

Maybe you could specify the dependency as ember-maybe-in-element: ^1.0.0 || ^2.0.0?

@maxfierke
Copy link
Collaborator

Is it ember-cli-dependency-lint that's forcing you to only use one or is there an error that arises in NPM or Yarn dependency resolution that causes issues?

Theoretically, since 1.0.0 is just a build-time AST transform, it seems like it should be usable alongside another version... (I would think, at least.)

We could do an OR-ed version constraint like that but I think both NPM and Yarn might default to picking the higher version, even on Ember versions its not compatible with, which I'd like to avoid if possible (since it would require some manual intervention for upgraders)

I may just inline the AST transform from 1.0.0

@andreyfel
Copy link
Author

andreyfel commented Aug 17, 2020

@maxfierke yes, it is ember-cli-dependency-lint which tells about versions conflict.

Anyway, the AST transform is not the best option since Glimmer components were introduced, see DockYard/ember-maybe-in-element#10 (comment)

@cibernox also added a runtime component implementation to the ember-maybe-in-element DockYard/ember-maybe-in-element#33

@cibernox
Copy link

To clarify, I implemented it as a mixed approach. There is a runtime component and an AST transforms that allows to invoke it as if it was a keyword instead of using the cumbersome <MaybeInElement @destinationElement={{}} @renderInPlace={{}}>

@maxfierke
Copy link
Collaborator

I opted to just remove the dependency. Released as 3.4.5

@andreyfel
Copy link
Author

Thanks @maxfierke!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants