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.

Sign up for GitHub

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

Jump to bottom

Add polyfill that allows apps and addons to use @ember-data related packages regardless of ember-data version #318

Merged
merged 5 commits into from
Jan 30, 2020
Merged

Add polyfill that allows apps and addons to use @ember-data related packages regardless of ember-data version #318

merged 5 commits into from
Jan 30, 2020

Conversation

runspired
Copy link
Contributor

No description provided.

rwjblue
rwjblue reviewed Jan 15, 2020
View reviewed changes
index.js Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@runspired runspired force-pushed the feat/ember-data-packages branch 2 times, most recently from 15574e4 to ead9fd6 Compare January 29, 2020 03:39
@runspired runspired force-pushed the feat/ember-data-packages branch from ead9fd6 to 58b4483 Compare January 29, 2020 03:43
@runspired runspired requested a review from rwjblue January 29, 2020 03:50
@runspired
Copy link
Contributor Author

runspired commented Jan 29, 2020 •
edited
Loading

The failing test is for node 6, which I believe we can safely drop.

Indeed this project currently tests 6,8, and 10 when we should bump to testing 10, 12 and 13

EDIT: @rwjblue requested the node 6/8 support drop be a separate effort later and not tied to any features. #321 has been created and node 6 support added here.

@amyrlam amyrlam mentioned this pull request Jan 29, 2020
Merged
27 tasks
rwjblue
rwjblue reviewed Jan 30, 2020
View reviewed changes
index.js
Comment on lines +531 to +533
if (!dep.version) {
throw new Error('EmberData missing version');
}
Copy link
Member

Choose a reason for hiding this comment

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

Do we ever expect this to actually happen? I think no, but just wanted to check and see why it was added...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I was creating the test mock environment the failures were pretty obtuse, this made it clear when the issue was no version found so that I knew I needed to continue getting the mock env closer to what version checker needed.

It is removable.

@rwjblue rwjblue changed the title feat: EmberData Packages Polyfill Add polyfill that allows apps and addons to use @ember-data related packages regardless of ember-data version Jan 30, 2020
@rwjblue rwjblue merged commit 2ba3ef7 into emberjs:master Jan 30, 2020
rwjblue added a commit that referenced this pull request Jan 30, 2020
@rwjblue
Revert "Add polyfill that allows apps and addons to use @ember-data…
f9de09f 
… re… (#318)"

This reverts commit 2ba3ef7, reversing
changes made to f305de0.

Seeing issues with this in the wild, reverting so that we can
investigate.
@amyrlam amyrlam mentioned this pull request Feb 1, 2020
Merged
18 tasks
runspired added a commit to runspired/ember-cli-babel that referenced this pull request Feb 4, 2020
@runspired
Revert "Revert "Add polyfill that allows apps and addons to use `@emb…
8b2b305 
…er-data` re… (emberjs#318)""

This reverts commit f9de09f.
@runspired runspired mentioned this pull request Feb 4, 2020
Merged
@snyk-bot snyk-bot mentioned this pull request Mar 25, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rwjblue rwjblue rwjblue left review comments

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@runspired @rwjblue