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

Deprecate Globals Resolver #331

Merged
merged 7 commits into from
Sep 7, 2018

Conversation

Gaurav0
Copy link
Contributor

@Gaurav0 Gaurav0 commented May 8, 2018

@Gaurav0 Gaurav0 changed the title Deprecate globals resolver Deprecate Globals esolver May 8, 2018
@Gaurav0 Gaurav0 changed the title Deprecate Globals esolver Deprecate Globals Resolver May 8, 2018
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

Definitely in favor of this, thanks for writing it up!


# Transition Path

Primarily, the transition path is to recommend using Ember-CLI.
Copy link
Member

Choose a reason for hiding this comment

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

Not sure anything needs to change here per-se, but using Ember without ember-cli is already officially not supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know. I mainly wrote this up so we could get some deprecation warnings set up.

@Gaurav0 Gaurav0 force-pushed the deprecate_globals_resolver branch 2 times, most recently from 3e78b7a to 50130c7 Compare May 10, 2018 20:25
@Gaurav0 Gaurav0 force-pushed the deprecate_globals_resolver branch from 50130c7 to 4d1c024 Compare May 10, 2018 20:25
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

We discussed this RFC during the Ember.js Core Team meeting today, and are moving it into the final comment period.

# Unresolved questions

There has never been a transition guide for transitioning an old codebase to Ember-CLI.
Do we want to create one at this late date?
Copy link
Member

Choose a reason for hiding this comment

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

I think that we certainly could do it, but I don't think that it would be a requirement for moving forward with this RFC. As it stands today, the migration path away from globals mode to a more natural ember-cli built application is something most of our community has already absorbed, and the effort of writing a thorough migration guide at this point far outweighs the upside...

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I think a traditional approach to deprecation RFCs is that we write the deprecation documentation here in the RFC. Once we pick a deprecation strategy I'd like to see that 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.

I would appreciate it if anyone can PR some documentation against this RFC.

I'm really not sure what to say except "Do not use globals resolver, use ember-cli".

Copy link
Member

Choose a reason for hiding this comment

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

@Gaurav0 put together a summary for the deprecation guide over in ember-learn/deprecation-app#153


# Transition Path

Primarily, the transition path is to recommend using Ember-CLI.
Copy link
Member

Choose a reason for hiding this comment

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

One small clarifying point for this transition path, is that ember-cli's own default resolver ember-resolver currently still extends from the globals resolver. In order to implement this RFC, and deprecate the globals resolver we need to change the ember-cli resolver so that it does not extend from the globals resolver (otherwise even ember-cli users would get a deprecation).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment added

@JackEllis
Copy link

I've never used this.

@andyhot
Copy link

andyhot commented Jun 19, 2018

Will the global resolver be moved to its own repo (once it's deprecated from ember)?

@rwjblue
Copy link
Member

rwjblue commented Jun 19, 2018

This RFC does not propose that, but it definitely seems possible if someone does like it / want it.

@Gaurav0
Copy link
Contributor Author

Gaurav0 commented Jun 20, 2018

@andyhot Are you using the globals resolver? How can we best help you transition?

Copy link
Sponsor Member

@mixonic mixonic left a comment

Choose a reason for hiding this comment

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

Thanks for pushing this forward @Gaurav0! I'm excited to see this cleanup happen.

currently still extends from the globals resolver.
In order to implement this RFC, the ember-cli resolver will need to be changed
so that it does *not* extend from the globals resolver, or otherwise ember-cli users
will get a deprecation warning as well.
Copy link
Sponsor Member

@mixonic mixonic Jun 26, 2018

Choose a reason for hiding this comment

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

We can't remove the global resolver base class for the ember-cli classic resolver, that would be a breaking change.

I can think of two options:

Simple option

Have the global resolver log a deprecation message at runtime. When resolve or any other method on the global resolver is hit, deprecation message. Then we remove a) the global resolver and b) ember-cli classic resolver's extension in 4.0.

Complex, faster option

In the ember-cli classic resolver, deprecate any runtime calls where there is fallback to the globals mode resolver. This would be a deprecation in ember-cli's resolver. We could bump a major version of ember-cli-resolver removing the base class and release it in ember-cli after an LTS of ember-cli.

Then separately (or concurrently with some cleverness) we could deprecate any use of the ember-cli globals resolver.

Copy link
Member

Choose a reason for hiding this comment

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

In the ember-cli classic resolver, deprecate any runtime calls where there is fallback to the globals mode resolver. This would be a deprecation in ember-cli's resolver. We could bump a major version of ember-cli-resolver removing the base class and release it in ember-cli after an LTS of ember-cli.

This was what I was planning on doing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unless there are any objections, I'll put this in the RFC?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, sounds good to update with that.


Over the past years we have transitioned to using Ember-CLI as the main way
to compile Ember apps. The globals resolver is a holdover and primarily
facilitates use of Ember without Ember-CLI.
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I did a quick review, and it does look like the ember-cli classic resolver replaces all methods on the globals resolver 👍

# Unresolved questions

There has never been a transition guide for transitioning an old codebase to Ember-CLI.
Do we want to create one at this late date?
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I think a traditional approach to deprecation RFCs is that we write the deprecation documentation here in the RFC. Once we pick a deprecation strategy I'd like to see that added.

Copy link
Contributor Author

@Gaurav0 Gaurav0 left a comment

Choose a reason for hiding this comment

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

@mixonic Thank you for your feedback.


# Transition Path

Primarily, the transition path is to recommend using Ember-CLI.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment added

currently still extends from the globals resolver.
In order to implement this RFC, the ember-cli resolver will need to be changed
so that it does *not* extend from the globals resolver, or otherwise ember-cli users
will get a deprecation warning as well.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unless there are any objections, I'll put this in the RFC?

# Unresolved questions

There has never been a transition guide for transitioning an old codebase to Ember-CLI.
Do we want to create one at this late date?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would appreciate it if anyone can PR some documentation against this RFC.

I'm really not sure what to say except "Do not use globals resolver, use ember-cli".

@ming-codes
Copy link

I think global resolver is still needed for the case where you can start using Ember by linking in via script tag. This was one of the selling point of Vue.

I didn’t know this is still possible before I read this rfc.

@mehulkar
Copy link
Contributor

mehulkar commented Jul 3, 2018

I've actually been looking forward to migrating a legacy app to Ember by way of the globals resolver (would be a nice onboarding strategy instead of doing a full rewrite). But that said, I can pretty trivially start with an older version of Ember to accomplish this.

@rtablada
Copy link
Contributor

rtablada commented Jul 3, 2018

@mehulkar are you looking to migrate a legacy Ember app or a server rendered site over to Ember? You can still use Ember to replace page by page without global resolver.

@ef4
Copy link
Contributor

ef4 commented Jul 3, 2018

I didn’t know this is still possible before I read this rfc.

For practical purposes, it's not. It's documented nowhere, it supports almost nothing you would want to add to your app, and we already dropped support for anyone not building with ember-cli back at 3.0.

If somebody wants to do a project to make a pleasant "try Ember with zero install" setup, that would be great. It's definitely doable (and Ember Twiddle already demonstrates most of what you'd need to get working). But it would not use the globals resolver, which -- far from being a nice selling point for Ember -- is part of the crufty history that we want to get rid of precisely because it makes adoption and innovation harder.

A nice Try Ember experience deserves to be designed for that purpose. The globals resolver is not it.

@Gaurav0
Copy link
Contributor Author

Gaurav0 commented Jul 11, 2018

I drafted a deprecation guide here: ember-learn/deprecation-app#155

@mixonic
Copy link
Sponsor Member

mixonic commented Jul 12, 2018

@Gaurav0 thanks for the deprecation draft.

FWIW, I think 90% of users who will run into this are not legacy non-ember-cli users. I believe we got those users to upgrade already. I believe it will be users of Ember-CLI who are using App.MyModel = and other globals items sprinkled through their otherwise modern codebase. Thus I think you could more simply tell people to "don't use App.FooBarComponent for example, instead create a file with the appropriate name per Ember documentation: app/components/foo-bar.js.

@Gaurav0 can you update your RFC to include a link to the deprecation guide draft? Thank you.

I expect we will FCP this on Friday and merge it next week!

@mehulkar
Copy link
Contributor

@rtablada it would have been a non-ember app. I would have tried to use it in exactly the way documented in this RFC (i.e. stick some script tags into my html that creates and renders an app in a specific div). There's no reason that I need to use latest Ember to try this out though.

@Gaurav0
Copy link
Contributor Author

Gaurav0 commented Jul 12, 2018

@mixonic
Copy link
Sponsor Member

mixonic commented Jul 12, 2018

@Gaurav0 to the pull request would be great. The idea is that someone reviewing the RFC should be able to read the proposed deprecation guidance either in the RFC itself or wherever it is elsewhere.

Thank you!

@Gaurav0
Copy link
Contributor Author

Gaurav0 commented Aug 2, 2018

FWIW, I think 90% of users who will run into this are not legacy non-ember-cli users. I believe we got those users to upgrade already. I believe it will be users of Ember-CLI who are using App.MyModel = and other globals items sprinkled through their otherwise modern codebase. Thus I think you could more simply tell people to "don't use App.FooBarComponent for example, instead create a file with the appropriate name per Ember documentation: app/components/foo-bar.js.

@mixonic I haven't run into this. In fact, most people don't know what the global namespace is for their app. Unless I made a mistake trying this, it doesn't work by default, even if they have a global namespace via https://github.com/ember-cli/ember-export-application-global and have enabled it in production mode. The ember-cli resolver takes over. Maybe one can get it working, but I'm not sure how.

@rwjblue
Copy link
Member

rwjblue commented Aug 31, 2018

Discussed with the core team again today, and we are 👍 on moving this back into final comment period.

@rwjblue
Copy link
Member

rwjblue commented Sep 7, 2018

👏 lets do it!

@rwjblue rwjblue merged commit a93e20a into emberjs:master Sep 7, 2018
@tomdale
Copy link
Member

tomdale commented Sep 7, 2018

Thank you @Gaurav0, and congrats on the merged RFC!

wagenet added a commit that referenced this pull request Mar 13, 2023
bmish added a commit to bmish/emberjs-rfcs that referenced this pull request May 21, 2023
* master: (56 commits)
  Fix code examples & add ember-cli release version in emberjs#637
  Update FCP guidance to include Discord
  Update RFC 085421 ready-for-release PR URL
  Advance RFC {{ inputs.rfc-number }} to Stage ready-for-release
  feat: EmberData Cache v2.1
  finalize lifetimes
  Update text/0860-ember-data-request-service.md
  Update RFC 496, typos, correct field name
  add note
  chore: update RequestService url with finalized design details
  Move emberjs#331 deprecate-globals-resolver to recommended
  Correct metadata for emberjs#487 custom model classes
  Move emberjs#625 helper-managers to recommended
  Add release date and version for 776
  Update RFC 0776 released PR URL
  Advance RFC {{ inputs.rfc-number }} to Stage released
  Update RFC 0739 ready-for-release PR URL
  Advance RFC {{ inputs.rfc-number }} to Stage ready-for-release
  Deprecate `ember-mocha`
  Add title of RFC to advancement PR titles
  ...
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.

10 participants