-
Notifications
You must be signed in to change notification settings - Fork 76
Ember 4.0 #267
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
base: master
Are you sure you want to change the base?
Ember 4.0 #267
Conversation
I attempted to leave the logic untouch as much as possible, converting mostly notation, with a few exceptions: - The `_onInit` event handler and `init` hooks had to be replaced by logic in the constructor and class fields. - `_init` method was renamed to `prepare` because it is overridden in test app's adapters and couldn't be made a true private method. Also, naming it `init` would collide with EmberObject hooks. - Made the constructor take the database object as parameter. Because `init` hooks runs after the constructor, this gave extending adapters enough time to set `this.db`. As we merged those into the constructor, the change handler would not be set, as `db` would not be defined yet. This is a breaking change. This commit also updates the test adapters to use this new constructor API.
… because it does not prove anything really.
…This is a breaking change.
…y be re-exportation of `PouchSerializer`. Instead of providing the application serializer in the addon, defines it only in the dummy app.
- Corrects ESLint config file name - dds rule to ignore variables starting with `_` - Prevents import linting errors in the ESLint config file itself
- Moves `PouchModel` to `models/pouch.js` - Exports the model from the new path - Adds a model export from `/app` - Exports `PouchDB`, with plugins already attached, directly from the addon
…ove away from old QUnit APIs. Also, that is makes the file intent more clear. Updates imports in test files to reflect this.
…gnoring unused arguments starting with `_`. Bumps JS version to latest.
…seless templating dependencies.
Thanks for all your work on this! I looked over the PR and all seems well, I’m not sure how we should proceed, this library hasn’t seen much activity in a while and I don’t think we have conventions for maintainers to follow. The lint job failed but based on the error it’s probably a Babel/transpiling thing 🤔 |
- Uses Node 18 in all jobs - Swaps yarn for pnpm - Updates try scenarios with the ones we actually provide
Thanks for the quick response! About the release plans, I think probably we should release two majors, one aiming v4 and other v5+. |
As numerous projects are running Ember 3 and 4 with Ember-Pouch 8.0.0-beta.2, and it is evident that Ember Data 3 is still used in Ember 4 projects, like for instance, LinkedIn uses Ember 4.8.6 and Ember Data 3.28.3 at the moment, indicating that Ember-Pouch should be compatible with both Ember 3 and 4. I am all for innovation like this, but I am not sure about the breaking changes. Since Ember-Pouch 8.0.0-beta.2 is currently used in Ember 4 projects, would it not be better to skip this innovation for Ember 4 and maintain compatibility with Ember 3? Maybe we can release the current Beta as 8.0.0 and make a breaking 9.0.0-beta.1 Ember 5+ with the Ember 5.0 #268 PR? |
@andsmedeiros See: https://github.com/broerse/ember-cli-blog . If we release 9.0.0-beta.1 for Ember 5+ It would be super if you have time to do a PR on ember-cli-blog to bring it to the newest Ember just to demo it. |
This commit provides support for Ember and EmberData 4.x.
What was done?
Besides resolving most deprecations and updating dependencies, this is a basis for the modernization of the codebase.
I attempted to migrate most old JS syntax to modern alternatives while maintaining functionality. Also, old Ember and Data paradigms were mostly shifted out, like EmberObject being ditched where possible. Some quirks and hacks for obsolete Ember/Data versions were removed.
What was not done?
I left out updating the README or the package version as that should be better handled by the package manager probably.
Also, this does NOT attempt to phase out EmberData's adapter paradigm yet.
Testing
I updated all the testing code also. However, I cannot get tests to run with scenarios
ember-lts-4.8
andember-lts-4.12
due to an error:This comes off some EmberData's private imports (IIRC
RecordArray
being undefined), but I couldn't spare the time to properly debug it.Other scenarios run fine.
Breaking changes
TL;DR
Before:
After:
Because
PouchAdapter
relied oninit()
to initialize its change listener,db
could be set as a class property, as that would be set beforeinit()
runs. Now, the change listener must be set on the adapter constructor and keeepingdb
as a class property would mean setting it only after the constructor runs.The adapter constructor have been now directly defined and takes the database object as parameter. This way, it can set the property itself and correctly set the change listener during construction.
Afterword
I will also open another PR with a branch aiming Ember and Data >=5 (including 6.0). I am available if anything is needed to getting this working well.
🖖🏽