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

Add compatibility for Ember 4 #208

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

ndekeister-us
Copy link

Breaking

Officially drop support for Node < 12

Drop support for Ember <3.24

Chore

Upgrade to Ember 4

  • run ember-cli-update --to 4.4.0 and yarn install

Also

  • specify node version supported in package.json (>= 12)
  • update ember-classic-decorator devDeps to v3.0.1 (v2 was not compatible with Ember 4)
  • update ember-data devDeps to 4.4.x (v3 was not compatible with Ember 4)
  • run yarn-deduplicate && yarn install to clean the yarn.lock
  • remove jshint config files used in tests (not relevant anymore, was introduced with first versions of Ember, we now use eslint in tests)
  • lint
    • fix some lint errors in test
    • fix template lint errors
    • ⚠️ temporarily move eslint and template lint error to a warning -> Goal of this PR is to make this repo compatible with Ember 4 so that consumers using Ember 4 can use it ; refactoring whole addon to native class / glimmer may introduce some side effects and it seems not a blocking for Ember 4

Remove object-bin export

Taken from #194 (file no longer exists in ./addon/component) should make embroider happy

Build

Remove ember-decorators from devDeps

It was not used by this repo

@ndekeister-us
Copy link
Author

Motivation

We are working on an old private project which is being migrated to Ember 4. At this moment v0.9.0 is not compatible with Ember 4, so it block us, this PR make ember-drag-drop compatible with Ember 4 and unblock us.

I'm not tackling refactor to glimmer and native class yet on this PR (only switched errors to warnings) because it could brings side effects and I don't have enough knowledge of this repo / our old repo to be sure I will detect them.

If this PR is

  • accepted 🎉 I will maybe take some times to work on these refactors and submit some PRs (would be happy to discuss with you @mharris717 , ndekeister#0721 in Ember Discord, we can maybe think about a roadmap / see if some other people are interested to work on it / contribute to this cool addon)
  • not accepted 😿 we will probably refactor our drag'n'drop implementation with another packages (but it could be nice to specify in the README.md that addon is not compatible with Ember 4 🙏 )

CI run -> https://github.com/ndekeister-us/ember-drag-drop/pull/1/checks (only embroider-optimized failing, didn't investigated

@ndekeister-us ndekeister-us marked this pull request as ready for review November 15, 2022 15:32
@boris-petrov
Copy link
Contributor

cc @dgavey

@dgavey
Copy link
Collaborator

dgavey commented Dec 14, 2022

Ok I'll take a look at it.

@tehhowch
Copy link

tehhowch commented Feb 2, 2023

bump - looking forward to seeing this work!

Copy link

@tehhowch tehhowch left a comment

Choose a reason for hiding this comment

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

When I yarn, I get warnings about missing a peer dep of @babel/core@^7, so that should probably be added here. Other than that, the changes seem sensible, given that the larger lint changes are marked TBD.

Were the existing lint changes from autofixing before the decision to defer that to a followup PR?

@IBRAHIMDANS
Copy link

When is this mr going to be merged ? @tehhowch

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.

5 participants