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

Ensure @ember/application/deprecations is stripped in addition to @ember/debug. #286

Merged
merged 6 commits into from
Jun 17, 2019

Conversation

bekzod
Copy link
Contributor

@bekzod bekzod commented Jun 13, 2019

@bekzod bekzod force-pushed the remove-deprecated-path branch 2 times, most recently from 317daf5 to e1d6f9b Compare June 13, 2019 09:12
@bekzod bekzod force-pushed the remove-deprecated-path branch from e1d6f9b to 828de36 Compare June 13, 2019 09:12
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
bekzod and others added 2 commits June 15, 2019 17:45
Co-Authored-By: Robert Jackson <[email protected]>
Co-Authored-By: Robert Jackson <[email protected]>
@bekzod
Copy link
Contributor Author

bekzod commented Jun 15, 2019

thank you @rwjblue, accepted all those changes

@rwjblue rwjblue changed the title remove deprecated import path Ensure @ember/application/deprecations is stripped in addition to @ember/debug. Jun 17, 2019
@rwjblue
Copy link
Member

rwjblue commented Jun 17, 2019

@bekzod - I've pushed a few changes:

  • added a failing test that attempts to confirm the babel-plugin-ember-modules-api-polyfill doesn't transpile import { deprecate } from '@ember/application/deprecations' to globals (e.g. Ember.deprecate)
  • fix that failing test (by ensuring that deprecate from @ember/application/deprecations is not transformed by the modules polyfill
  • added a test confirming that import { deprecate } from '@ember/application/deprecations is transpiled properly in debug builds

Should be good to land once CI is finished...

@rwjblue rwjblue merged commit 61f65e7 into emberjs:master Jun 17, 2019
@bekzod bekzod deleted the remove-deprecated-path branch June 17, 2019 19:57

return [[require.resolve('babel-plugin-debug-macros'), options]];
[
require.resolve('babel-plugin-debug-macros'),
Copy link
Member

Choose a reason for hiding this comment

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

running the same plugin twice seems unfortunate, can the primary plugin simply be taught how to handle this case?

Copy link
Member

Choose a reason for hiding this comment

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

We already support arbitrary module paths with flags, but not with debugTools. We can definitely update babel-plugin-debug-macros to support that though.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

investigating

Copy link
Member

Choose a reason for hiding this comment

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

The issue seems to have been a monorepo with a shared lockfile resulted in an incomplete semver drift of relevant dependencies, upgrading all repos contained within the monorepo fixed the issues.

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

Successfully merging this pull request may close these issues.

3 participants