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

Remove jQuery by default #386

Merged
merged 4 commits into from
Jan 11, 2019
Merged

Remove jQuery by default #386

merged 4 commits into from
Jan 11, 2019

Conversation

simonihmig
Copy link
Contributor

@simonihmig simonihmig commented Oct 7, 2018

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Oct 7, 2018

This is probably blocking the jQuery removal endeavor emberjs/data#5386

@tavosansal
Copy link

tavosansal commented Oct 7, 2018

So how would one select an element inside a component? Using this.$ was convinient because it scoped the html of that component. Will there be a way to do that without jQuery?

@makepanic
Copy link
Contributor

So how would one select an element inside a component? Using this.$ was convinient because it scoped the html of that component. Will there be a way to do that without jQuery?

Components have a element property. You can refactor this.$() to something like this.element.querySelector() (afaik this only works for components with a tagName)

@rwjblue
Copy link
Member

rwjblue commented Oct 7, 2018

afaik this only works for components with a tagName

Confirm, however in tagless components you also don’t have a this.$() so this isn’t a new constraint / change.

@tavosansal
Copy link

Makes sense...I did not know about the element property. Might be good to note to change the docs about this then and start teaching about it instead of the this.$ as present here: https://guides.emberjs.com/release/components/the-component-lifecycle/#toc_integrating-with-third-party-libraries-with-didinsertelement

@simonihmig
Copy link
Contributor Author

Might be good to note to change the docs about this

@tavosansal Indeed, tbh I wasn't expecting to still see references to jQuery APIs in the docs. Even as part of the previous optional jQuery RFC it makes sense to only teach native APIs, especially for addons! Created this tracking issue: ember-learn/guides-source#194

@simonihmig
Copy link
Contributor Author

simonihmig commented Oct 8, 2018

This is probably blocking the jQuery removal endeavor emberjs/data#5386

Just for the record: the ember-fetch adapter mixin already supports using fetch with ember-data, so you can have an ember-data app without jQuery even today. Though I agree it would be nice to have support built-in. But I would expect this to land in ember-data at least by the time, probably way before this RFC gets implemented (should it be accepted), so hopefully no real blocker!?

@jherdman
Copy link

jherdman commented Oct 8, 2018

It should be noted that ember-cli-mirage doesn't yet support fetch as it's stuck on Pretender ~1.6. This might be considered a blocker given how popular it is as a development tool.

@NullVoxPopuli
Copy link
Contributor

I'd say that'd force a major version of mirage than anything else

@initram
Copy link

initram commented Oct 17, 2018

We should also consider the use of ember-ajax as it depends on jquery. ember-ajax is currently part of the default blueprint, and apps/addons may use that instead of using jquery $ directly. I think this should be reflected in the RFC, so that we don't end up having addons that use jquery in the blueprint after we remove jquery by default.

tylerturdenpants added a commit to emberjs/website that referenced this pull request Oct 19, 2018
Ideas, feel free to add to list or claim:
- [ ] ```I've been getting a lot of questions about how tree-shaking is coming along. I would be willing to train anyone that wants to help on what's already done and what still needs to be done. Disclaimer: It's a lot of work! #emberjs #EmberFest``` https://twitter.com/kellyselden/status/1050717338595745792
- [ ] Include summary from pixelhandler (and cc him) #issue-triage team: https://discordapp.com/channels/480462759797063690/480523776845414412/500377829452546058
- [x] From jenweber on #support-ember-times: Another times topic: emberjs/ember.js#16910 I would summarize as, "ember.js has an addition to the test suite to help with API documentation! In the past, you might have noticed that a small number of methods  had missing docs. When code gets moved around in ember.js, such as putting functions in their own modules, it's easy to make mistakes that impact the documentation parser. This test builds the docs and checks them for unintentional changes. Thanks <link to ed> for the test and to everyone who helped fix missing docs over the years." Todd Jordan did the majority of the fixes when things got dropped, I think. Hard to say for sure 🔏 @kennethlarsen 
- [x] emberjs/rfcs#384 (:lock: @jessica-jordan)
- [x] emberjs/rfcs#386 (:lock: @jessica-jordan)
- [ ] emberjs/rfcs#387 (:lock: @jessica-jordan)
- [x] emberjs/rfcs#388 (:lock: @Alonski)
- [ ] emberjs/rfcs#389
- [x] EmberConf brainstorming session date, if confirmed (:lock: @amyrlam)
- [ ] Hacktoberfest roundup?
- [ ] Check https://github.com/jessica-jordan/whats-new-in-emberland
- [x] [ember-self-focused](https://engineering.linkedin.com/blog/2018/10/making-ember-applications--ui-transitions-screen-reader-friendly) (🔒 @chrisrng )
- [ ] ...

Ideas we are waiting on:
- [ ] EmberCamp talks, deep dive? http://embercamp.com/ and https://github.com/ember-chicago/ember-camp-2018-notes ... Maybe we wait for the talk videos to be published? Keep an eye on #ember-camp in Discord. not out yet
- [ ] GraphQL with Ember? https://emberfest.eu/schedule/#rocky-neurock Or maybe we can reach out to them for a post-EmberFest writeup? See also the blog post: https://medium.com/kloeckner-i/ember-and-graphql-8aa15f7a2554
@rwjblue rwjblue self-assigned this Oct 24, 2018
@rwjblue rwjblue added the T-framework RFCs that impact the ember.js library label Oct 24, 2018
@jenweber
Copy link
Contributor

The "How We Teach This" could mention the work that you've already done to extract JQuery examples from the docs, the jquery migration guide, and the Linting rules that are now in place.

@simonihmig
Copy link
Contributor Author

It should be noted that ember-cli-mirage doesn't yet support fetch as it's stuck on Pretender ~1.6.

This isn't the case anymore, it is using pretender 2.1.0, so it should support (native) fetch now. So one less thing to worry about!

@simonihmig
Copy link
Contributor Author

I just updated the RFC to incorporate the feedback...

  • added a section to update the ember new blueprint, mentioning to replace ember-ajax with ember-fetch and enable the no-jquery ESLint rule by default
  • mention the remaining ember-data "blocker" to support ember-fetch out of the box
  • updates to "How We Teach This"

Any word from the Core team on this? If going forward with this, I think it would be good to get the deprecation messages out asap, so users/addon authors have enough time to migrate, before dropping support in the next major version. Especially as IMHO this could be regarded as part of or at least somewhat related to Octane...

@rwjblue
Copy link
Member

rwjblue commented Dec 23, 2018

We reviewed this in detail at the most recent face to face meeting (sorry for the long delay getting this feedback posted). In general, we are very much in favor of moving forward!

There are a few areas where we think the RFC needs to be fleshed out a bit:

  • We need to include a plan to ensure new applications work out of the box without jQuery, this means that we need to address any of the default packages that an application ships with that requires jQuery. Specifically:
    • ember-data requires jQuery.ajax by default
    • ember-ajax would need to be removed from the default blueprint
  • Ember.$ should always be deprecated (even when using @ember/jquery)
  • moduleForComponent should not be modified (these "legacy" test modes are "frozen", and will be deprecated completely in a separate RFC, there is no point causing undue churn to satisify this.$ deprecation in them (you should just migrate to the new syntax))
  • We need to review the ember-inspector to ensure that it is fully functional without jQuery (I believe that it works well and all features are present, but I'd like for the RFC to confirm)
  • The RFC should be updated to include the deprecation guide data (or at least cross link to a deprecation guide app PR)
  • Some specific mention of uses of jQuery.ajax with prefilter should be included in the RFC. (I suspect this will be a somewhat tricky one to solve, and may result in a new service in ember-fetch...)

Since we are considering this RFC as part of the Octane Edition, we are very interested in moving forward quickly...

@cibernox
Copy link
Contributor

@rwjblue I confirm that the ember-inspector works in apps and addons without jquery.

@simonihmig
Copy link
Contributor Author

@rwjblue updated again according to your feedback. Should be good for another review!

We need to include a plan to ensure new applications work out of the box without jQuery, this means that we need to address any of the default packages that an application ships with that requires jQuery. Specifically:

  • ember-data requires jQuery.ajax by default
  • ember-ajax would need to be removed from the default blueprint

Had this already covered in an update a few days ago, just did some further refinement.

Ember.$ should always be deprecated (even when using @ember/jquery)

Makes sense. Updated the RFC accordingly.

moduleForComponent should not be modified (these "legacy" test modes are "frozen", and will be deprecated completely in a separate RFC, there is no point causing undue churn to satisify this.$ deprecation in them (you should just migrate to the new syntax))

Same, has been updated.

We need to review the ember-inspector to ensure that it is fully functional without jQuery (I believe that it works well and all features are present, but I'd like for the RFC to confirm)

@cibernox confirmed this, which matches my experience as well. I am not aware of any shortcomings so far.

The RFC should be updated to include the deprecation guide data (or at least cross link to a deprecation guide app PR)

As we have discussed, I added a bit of direction regarding the message and tone of the deprecation guide, and believe we can flesh this out during the implementation phase.

Some specific mention of uses of jQuery.ajax with prefilter should be included in the RFC. (I suspect this will be a somewhat tricky one to solve, and may result in a new service in ember-fetch...)

Has been added (without going into details API-wise).

@rwjblue
Copy link
Member

rwjblue commented Jan 3, 2019

Thank you @simonihmig, I've added this to the agenda for this Fridays core team meeting...

@rwjblue
Copy link
Member

rwjblue commented Jan 4, 2019

Discussed with the rest of the core team at today's meeting, and we are happy to move this into final comment period.

text/0000-remove-jquery.md Outdated Show resolved Hide resolved
text/0000-remove-jquery.md Outdated Show resolved Hide resolved
@rwjblue
Copy link
Member

rwjblue commented Jan 11, 2019

Given that this has been in FCP for a week, we reviewed it again at the core team meeting today and are still in favor of moving forward.

Thanks to everyone for helping us hone the message, and thanks to @simonihmig for pushing it forward!

@rwjblue rwjblue merged commit 3324cb8 into emberjs:master Jan 11, 2019
@simonihmig simonihmig deleted the remove-jquery branch January 11, 2019 21:51
@simonihmig simonihmig restored the remove-jquery branch January 12, 2019 11:07
buschtoens pushed a commit to ClarkSource/ember-css-modules-config that referenced this pull request Apr 3, 2019
Bumps [ember-source](https://github.com/emberjs/ember.js) from 3.8.0 to 3.9.0.
<details>
<summary>Release notes</summary>

*Sourced from [ember-source's releases](https://github.com/emberjs/ember.js/releases).*

> ## v3.9.0-beta.5
> ### CHANGELOG
> 
> - [#17733](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17733) [BUGFIX] Assert on use of reserved component names (`input` and `textarea`)
> 
> ## v3.9.0-beta.4
> ### CHANGELOG
> 
> - [#17710](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17710) [BUGFIX] Allow accessors in mixins
> 
> ## v3.9.0-beta.3
> ### CHANGELOG
> 
> - [#17684](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17684) [BUGFIX] Enable maximum rerendering limit to be customized.
> - [#17691](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17691) [BUGFIX] Ensure tagForProperty works on class constructors
> 
> ## v3.9.0-beta.2
> ### CHANGELOG
> 
> - [#17618](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17618) [BUGFIX] Migrate autorun microtask queue to Promise.then
> - [#17649](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17649) [BUGFIX] Revert decorator refactors
> 
> ## v3.9.0-beta.1
> ### CHANGELOG
> 
> 
> - [#17470](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17470) [DEPRECATION] Implements the Computed Property Modifier deprecation RFCs
>   * Deprecates `.property()` (see [emberjs/rfcs#375](https://github.com/emberjs/rfcs/blob/master/text/0375-deprecate-computed-property-modifier.md)
>   * Deprecates `.volatile()` (see [emberjs/rfcs#370](https://github.com/emberjs/rfcs/blob/master/text/0370-deprecate-computed-volatile.md)
>   * Deprecates computed overridability (see [emberjs/rfcs#369](https://github.com/emberjs/rfcs/blob/master/text/0369-deprecate-computed-clobberability.md) 
> - [#17488](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17488) [DEPRECATION] Deprecate this.$() in curly components (see [emberjs/rfcs#386](https://github.com/emberjs/rfcs/blob/master/text/0386-remove-jquery.md))
> - [#17489](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17489) [DEPRECATION] Deprecate Ember.$() (see [emberjs/rfcs#386](https://github.com/emberjs/rfcs/blob/master/text/0386-remove-jquery.md))
> - [#17540](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17540) [DEPRECATION] Deprecate aliasMethod
> - [#17487](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17487) [BUGFIX] Fix scenario where aliased properties did not update in production mode
</details>
<details>
<summary>Changelog</summary>

*Sourced from [ember-source's changelog](https://github.com/emberjs/ember.js/blob/master/CHANGELOG.md).*

> # Ember Changelog
> 
> ### v3.9.0-beta.5 (March 25, 2019)
> 
> - [#17733](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17733) [BUGFIX] Assert on use of reserved component names (`input` and `textarea`)
> 
> ### v3.9.0-beta.4 (March 11, 2019)
> 
> - [#17710](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17710) [BUGFIX] Allow accessors in mixins
> 
> ### v3.9.0-beta.3 (March 4, 2019)
> 
> - [#17684](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17684) [BUGFIX] Enable maximum rerendering limit to be customized.
> - [#17691](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17691) [BUGFIX] Ensure tagForProperty works on class constructors
> 
> ### v3.9.0-beta.2 (February 26, 2019)
> 
> - [#17618](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17618) [BUGFIX] Migrate autorun microtask queue to Promise.then
> - [#17649](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17649) [BUGFIX] Revert decorator refactors
> 
> ### v3.9.0-beta.1 (February 18, 2019)
> 
> - [#17470](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17470) [DEPRECATION] Implements the Computed Property Modifier deprecation RFCs
>   * Deprecates `.property()` (see [emberjs/rfcs#375](https://github.com/emberjs/rfcs/blob/master/text/0375-deprecate-computed-property-modifier.md)
>   * Deprecates `.volatile()` (see [emberjs/rfcs#370](https://github.com/emberjs/rfcs/blob/master/text/0370-deprecate-computed-volatile.md)
>   * Deprecates computed overridability (see [emberjs/rfcs#369](https://github.com/emberjs/rfcs/blob/master/text/0369-deprecate-computed-clobberability.md) 
> - [#17488](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17488) [DEPRECATION] Deprecate this.$() in curly components (see [emberjs/rfcs#386](https://github.com/emberjs/rfcs/blob/master/text/0386-remove-jquery.md))
> - [#17489](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17489) [DEPRECATION] Deprecate Ember.$() (see [emberjs/rfcs#386](https://github.com/emberjs/rfcs/blob/master/text/0386-remove-jquery.md))
> - [#17540](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17540) [DEPRECATION] Deprecate aliasMethod
> - [#17487](https://github-redirect.dependabot.com/emberjs/ember.js/pull/17487) [BUGFIX] Fix scenario where aliased properties did not update in production mode
</details>
<details>
<summary>Commits</summary>

- [`8df20e9`](emberjs/ember.js@8df20e9) Release v3.9.0
- [`2b9ee17`](emberjs/ember.js@2b9ee17) Add v3.9.0 to CHANGELOG
- [`bfe670c`](emberjs/ember.js@bfe670c) Bump router_js
- [`2f8b8ba`](emberjs/ember.js@2f8b8ba) [DOCS beta] remove component nesting docs
- [`0270001`](emberjs/ember.js@0270001) Release v3.9.0-beta.5
- [`1e2672c`](emberjs/ember.js@1e2672c) Add v3.9.0-beta.5 to CHANGELOG
- [`37d0a11`](emberjs/ember.js@37d0a11) Fix docs test
- [`86999a7`](emberjs/ember.js@86999a7) Post-cherry-pick lint fixup
- [`fff729a`](emberjs/ember.js@fff729a) [DOC RouteInfo] Fix grammar, spelling, and formatting
- [`b179dfd`](emberjs/ember.js@b179dfd) added param "name of the block" for the API document of has-block and has-blo...
- Additional commits viewable in [compare view](emberjs/ember.js@v3.8.0...v3.9.0)
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=ember-source&package-manager=npm_and_yarn&previous-version=3.8.0&new-version=3.9.0)](https://dependabot.com/compatibility-score.html?dependency-name=ember-source&package-manager=npm_and_yarn&previous-version=3.8.0&new-version=3.9.0)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
If all status checks pass Dependabot will automatically merge this pull request during working hours.

[//]: # (dependabot-automerge-end)

---

**Note:** This repo was added to Dependabot recently, so you'll receive a maximum of 5 PRs for your first few update runs. Once an update run creates fewer than 5 PRs we'll remove that limit.

You can always request more updates by clicking `Bump now` in your [Dependabot dashboard](https://app.dependabot.com).

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot ignore this [patch|minor|major] version` will close this PR and stop Dependabot creating any more for this minor/major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in the `.dependabot/config.yml` file in this repo:
- Update frequency (including time of day and day of week)
- Automerge options (never/patch/minor, and dev/runtime dependencies)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)

Finally, you can contact us by mentioning @dependabot.

</details>

[//]: # (dependabot-no-ci-start)

---
⚠️ **Dependabot won't automerge this PR as it didn't detect CI on it** ⚠️ 

You have automerging enabled for this repo but Dependabot didn't detect any CI statuses or checks. You can disable automerging on this repo by editing the `.dependabot/config.yml` file in this repo.

[//]: # (dependabot-no-ci-end)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Final Comment Period T-framework RFCs that impact the ember.js library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants