From 66bfb7d03e9b6e7b8fd57792d1268334e02fc73a Mon Sep 17 00:00:00 2001 From: Gabriel Csapo Date: Mon, 23 Nov 2020 13:39:18 -0800 Subject: [PATCH 1/6] Deprecate Fallback Lookup Paths in ember-resolver --- ...cate-fallback-resolution-ember-resolver.md | 139 ++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 text/0683-deprecate-fallback-resolution-ember-resolver.md diff --git a/text/0683-deprecate-fallback-resolution-ember-resolver.md b/text/0683-deprecate-fallback-resolution-ember-resolver.md new file mode 100644 index 0000000000..13fa8d6cbd --- /dev/null +++ b/text/0683-deprecate-fallback-resolution-ember-resolver.md @@ -0,0 +1,139 @@ +--- +Start Date: 2020-11-23 +Relevant Team(s): Ember.js, Learning +RFC PR: https://github.com/emberjs/rfcs/pull/683 +Tracking: (leave this empty) +--- + +# Deprecate Fallback Lookup Paths in ember-resolver + +## Summary + +Currently we have multiple fallback lookup paths in ember-resolver. As we continue to migrate towards a more strict future where imports are verified at build time (https://github.com/emberjs/rfcs/pull/454) it will be helpful to remove non-strict lookups from ember-resolver, which support many-to-one lookups, in favor of a one-to-one mechanism. This will enable us to make codemods to migrate to: +Strict Mode (https://github.com/emberjs/rfcs/pull/496) +the v2 addon format (https://github.com/emberjs/rfcs/pull/507) +the slimming down the ember-cli build pipeline (https://github.com/emberjs/rfcs/pull/578) +strict service lookups (https://github.com/emberjs/rfcs/pull/585) + +## Motivation + +As the community is moving forward with strict lookups having this done in ember-resolver will allow developers to slowly migrate libraries and applications to take advantage of imports verified at build time, the v2 addon format, Strict Mode and Strict Service Lookups. Without ember-resolver support to ensure that applications and libraries use strict lookups, migrating the ecosystem will require application developers using new features to discover issues when integration occurs. + +Switching to the strict resolver by default and deprecating the classic resolver opens a migration path for applications that want to ensure compatibility with the new v2 package format. Additionally, apps using the strict resolver will see significant boot-up performance wins, because lookup costs become O(1) instead of O(n). + +Ember apps and addons currently default to using https://github.com/ember-cli/ember-resolver to lookup values from the registry. These values include: +Services +Components +Helpers +Modifiers +Templates +ember-strict-resolver defines a stricter and therefore much faster version of these lookups, which results in a 1:1 instead of 1:N resolution. + +In the flagship web app at LinkedIn, adopting ember-strict-resolver resulted in the following improvements (per a TracerBench analysis): + +boot phase estimated difference -188ms [-195ms to -181ms] +transition phase estimated difference -42ms [-57ms to -27ms] +render phase estimated difference -23ms [-37ms to -9ms] + +## Detailed design + +As libraries migrate to the new use strict format older applications will not be negatively affected as the strict subset resolution is backwards compatible from libraries to applications. + +The major difference between the two resolvers can be easily linted against: + +_It is important to call out that app re-exports are necessary when referencing components coming from an external source or using Wall Street Syntax._ + +| Lookup | Ember-resolver | ember-strict-resolver | +| ------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------- | +| service/foo‌ ‌ | service:foo‌ ‌ | service:foo‌ ‌ | +| service/foo-bar‌ ‌ | service:foo-bar,‌ ‌service:fooBar‌ ‌ | service:foo-bar‌ ‌ | +| (external-module-name)/addo‌n/services/foo-bar‌ ‌ | service:(external-module-nam‌e)@foo-bar‌ ‌service:(external-module-name)@fooBar‌ | service:(external-module-nam‌e)@foo-bar‌ ‌ | +| controller/foo/bar‌ ‌ | this.controllerFor(‘foo.bar’),‌ ‌this.controllerFor(‘foo/bar’)‌ ‌ | this.controllerFor(‘foo.bar’)‌ ‌ | +| component/foo/bar‌ ‌ | ‌{{foo/bar}}‌ ‌ | {{foo/bar}}‌ ‌ | +| component/foo/bar-baz‌ ‌ | ‌{{foo/bar-baz}},‌ ‌{{foo/bar_baz}}‌ ‌ | ‌{{foo/bar-baz}}‌ ‌ | +| (external-module-name)/addo‌n/components/foo/bar-baz‌ ‌ | {{component‌ ‌“(external-module-name)/foo/‌bar-baz”}}‌ ‌{{component‌ ‌“(external-module-name)/foo/‌bar_baz”}}‌ ‌{{component‌ ‌“(external-module-name)/foo/‌barBaz”}}‌ ‌ ‌ | {{component‌ ‌“(external-module-name)/foo/‌bar-baz”}}‌ ‌ | + +### Timeline + +Make the changes to turn on the strict mode lookups in ember-resolver under a flag + +As mentioned above turning on the strict mode should be added as a patch update to ember-resolver with a deprecation message that it will be turned on as a default operation in the next major version. This option would be set in the `ember-cli-build.js` for ember-resolver as the following: + +> A reason for making this option as a part of ember-cli-build.js will allow the build system to only bundle the strict resolver. This could be argued as a limitation for toggling between a strict and non-strict mode. + +``` + "ember-resolver": { + strict: true, + } +``` + +Make the strict resolver the default for new applications and addons. + +Making the resolver default to be turned on in strict mode in the default ember addon or application template is important as new addons being created should have an eye towards compatibility with the new system. + +Document how to migrate to strict resolution. + +This will have to be done in ember-resolver and in the ember guides to ensure that all new addons have the correct information when adding new content to the ecosystem and for those migrating. Lint rules can be added to eslint-plugin-ember as well as ember-template-lint to ensure that addons and applications that are interested in migrating are able to gauge the requirements. There will need to be additional tooling added in ember-cli or in ember-resolver to understand if any downstream dependencies of an application or addon do not support strict mode. As stated above leaf nodes migrating to strict mode do not impact upstream dependencies ability to consume them but if upstream dependencies update before downstream dependencies there is a possibility for runtime issues (these will be caught in tests as lookups will just fail to resolve). + +Deprecate the loose-mode resolver. + +Deprecating the loose-model in ember-resolver should happen in the next major version to avoid large ecosystem repercussions due to capability issues. + +### Questions + +can an app turn on the strict resolver before all of its addons have migrated? +can addons switch to using the strict resolver internally and still be consumed by an app using the classic resolver? + +## How we teach this + +_What names and terminology work best for these concepts and why? How is this +idea best presented? As a continuation of existing Ember patterns, or as a +wholly new one?_ + +This change actually reduces the friction for both new and existing users, since currently there are multiple ways to access the registry. Older codebases exhibit a mix of syntax and legacy lookup logic, which will be cleaned up by adopting the strict resolver. + +_Would the acceptance of this proposal mean the Ember guides must be +re-organized or altered? Does it change how Ember is taught to new users_ +at any level? + +The current guides currently adhere to a strict lookup policy from my searching. To guarantee it does not regress, the Ember guides tutorials migrate to the strict flag in ember-resolver such as https://github.com/ember-learn/super-rentals/tree/super-rentals-tutorial-output. + +_How should this feature be introduced and taught to existing Ember +users?_ + +The gradual migration would be an opt-in flag introduced in a minor version of https://github.com/ember-cli/ember-resolver and then turned on as the default in the major version after that. + +## Drawbacks + +> Why should we _not_ do this? Please consider the impact on teaching Ember, +> on the integration of this feature with other existing and planned features, +> on the impact of the API churn on existing apps, etc. + +A reason to not move the ecosystem forward in this regard is to prioritize technologies that will remove the need for the registry as a sub-system of Ember all-together. Technologies like template imports and reworking the service injection system to not require a resolution mechanic to function. + +This migration would introduce a level of churn to the ecosystem as it requires all libraries and applications to become more strict with their resolution, but does benefit the tightening of the interface to interact with the registry for potentially more functionality in the future such as Strict Service Lookups. + +This is an iterative performance win for the ecosystem. Future enhancements such as Template Imports will unlock additional wins on top of this but not gate boot performance benefits. + +## Alternatives + +> What other designs have been considered? What is the impact of not doing this? + +Don't do this, and just make the jump to full strict mode to unlock performance wins. This would mean that all performance improvements are gated by the use of strict-mode templates, rather than something that can be done incrementally. Not reducing the surface area for dynamic lookups will force the ecosystem to adopt template imports for all their dependencies to unlock performance benefits. + +> This section could also include prior art, that is, how other frameworks in the same domain have solved this problem. + +Major frameworks (React, Vue, Svelte, etc) ecosystems do not do dynamic lookup the way Ember does, but implore the use of node resolution to get templates or components. Ember’s path towards this solution comes with the use of template imports and the Embroider packaging v2 spec. Reducing the many to one lookup and eventually migrating from the dynamic lookup cases for components and templates to use node resolution via template imports and the v2 packaging spec will allow the ecosystem to gradually migrate. + +## Unresolved questions + +> Optional, but suggested for first drafts. What parts of the design are still +> TBD? + +What is the migration path and support for pods? + +## Appendix + +This document was written based on debugging how often we call into the resolver to access values from the registry, on an initial page load of our homepage we access the registry ~11,800 times. +We utilized initial findings from running the no-implicit-this lint rule to estimate our migration effort, and then ran the no-implicit-this codemod gradually across the codebase. As a side benefit, this helped us in running the angle-brackets codemod . +A footgun we found when running the migration was ensuring that our data-test-selector instances used in curly components to be implicit booleans which we found by helping contribute https://github.com/ember-template-lint/ember-template-lint/blob/master/docs/rule/no-positional-data-test-selectors.md and running the fixer across our codebase. From 56eef7d84fb12c12677f7d43b7b7f89cd0cc9fc1 Mon Sep 17 00:00:00 2001 From: Gabriel Csapo Date: Tue, 12 Jan 2021 16:55:38 -0800 Subject: [PATCH 2/6] Update text/0683-deprecate-fallback-resolution-ember-resolver.md Co-authored-by: Robert Jackson --- text/0683-deprecate-fallback-resolution-ember-resolver.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0683-deprecate-fallback-resolution-ember-resolver.md b/text/0683-deprecate-fallback-resolution-ember-resolver.md index 13fa8d6cbd..fab64eb4de 100644 --- a/text/0683-deprecate-fallback-resolution-ember-resolver.md +++ b/text/0683-deprecate-fallback-resolution-ember-resolver.md @@ -29,7 +29,7 @@ Modifiers Templates ember-strict-resolver defines a stricter and therefore much faster version of these lookups, which results in a 1:1 instead of 1:N resolution. -In the flagship web app at LinkedIn, adopting ember-strict-resolver resulted in the following improvements (per a TracerBench analysis): +In the flagship web app at LinkedIn, adopting ember-strict-resolver resulted in the following improvements (per a [TracerBench](https://www.tracerbench.com/) analysis): boot phase estimated difference -188ms [-195ms to -181ms] transition phase estimated difference -42ms [-57ms to -27ms] From c9c1122bf297ae1985a19acd40662ae1b76272bb Mon Sep 17 00:00:00 2001 From: Gabriel Csapo Date: Thu, 14 Jan 2021 10:02:03 -0800 Subject: [PATCH 3/6] Update text/0683-deprecate-fallback-resolution-ember-resolver.md Co-authored-by: Robert Jackson --- text/0683-deprecate-fallback-resolution-ember-resolver.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/text/0683-deprecate-fallback-resolution-ember-resolver.md b/text/0683-deprecate-fallback-resolution-ember-resolver.md index fab64eb4de..d17b7e5312 100644 --- a/text/0683-deprecate-fallback-resolution-ember-resolver.md +++ b/text/0683-deprecate-fallback-resolution-ember-resolver.md @@ -10,10 +10,10 @@ Tracking: (leave this empty) ## Summary Currently we have multiple fallback lookup paths in ember-resolver. As we continue to migrate towards a more strict future where imports are verified at build time (https://github.com/emberjs/rfcs/pull/454) it will be helpful to remove non-strict lookups from ember-resolver, which support many-to-one lookups, in favor of a one-to-one mechanism. This will enable us to make codemods to migrate to: -Strict Mode (https://github.com/emberjs/rfcs/pull/496) -the v2 addon format (https://github.com/emberjs/rfcs/pull/507) -the slimming down the ember-cli build pipeline (https://github.com/emberjs/rfcs/pull/578) -strict service lookups (https://github.com/emberjs/rfcs/pull/585) +* Strict Mode (https://github.com/emberjs/rfcs/pull/496) +* the v2 addon format (https://github.com/emberjs/rfcs/pull/507) +* the slimming down the ember-cli build pipeline (https://github.com/emberjs/rfcs/pull/578) +* strict service lookups (https://github.com/emberjs/rfcs/pull/585) ## Motivation From de6ecf9f6ec01c8cfefec366ca2668fa21eff836 Mon Sep 17 00:00:00 2001 From: Gabriel Csapo Date: Thu, 14 Jan 2021 10:02:09 -0800 Subject: [PATCH 4/6] Update text/0683-deprecate-fallback-resolution-ember-resolver.md Co-authored-by: Robert Jackson --- text/0683-deprecate-fallback-resolution-ember-resolver.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/text/0683-deprecate-fallback-resolution-ember-resolver.md b/text/0683-deprecate-fallback-resolution-ember-resolver.md index d17b7e5312..56cb34a713 100644 --- a/text/0683-deprecate-fallback-resolution-ember-resolver.md +++ b/text/0683-deprecate-fallback-resolution-ember-resolver.md @@ -31,9 +31,9 @@ ember-strict-resolver defines a stricter and therefore much faster version of th In the flagship web app at LinkedIn, adopting ember-strict-resolver resulted in the following improvements (per a [TracerBench](https://www.tracerbench.com/) analysis): -boot phase estimated difference -188ms [-195ms to -181ms] -transition phase estimated difference -42ms [-57ms to -27ms] -render phase estimated difference -23ms [-37ms to -9ms] +* `boot` phase estimated difference -188ms [-195ms to -181ms] +* `transition` phase estimated difference -42ms [-57ms to -27ms] +* `render` phase estimated difference -23ms [-37ms to -9ms] ## Detailed design From 1ad771612bcb40803a7ab60bf5495351e8444d35 Mon Sep 17 00:00:00 2001 From: Gabriel Csapo Date: Thu, 14 Jan 2021 10:02:27 -0800 Subject: [PATCH 5/6] Update text/0683-deprecate-fallback-resolution-ember-resolver.md Co-authored-by: Robert Jackson --- ...3-deprecate-fallback-resolution-ember-resolver.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/text/0683-deprecate-fallback-resolution-ember-resolver.md b/text/0683-deprecate-fallback-resolution-ember-resolver.md index 56cb34a713..5f3a1e4bfc 100644 --- a/text/0683-deprecate-fallback-resolution-ember-resolver.md +++ b/text/0683-deprecate-fallback-resolution-ember-resolver.md @@ -22,11 +22,13 @@ As the community is moving forward with strict lookups having this done in ember Switching to the strict resolver by default and deprecating the classic resolver opens a migration path for applications that want to ensure compatibility with the new v2 package format. Additionally, apps using the strict resolver will see significant boot-up performance wins, because lookup costs become O(1) instead of O(n). Ember apps and addons currently default to using https://github.com/ember-cli/ember-resolver to lookup values from the registry. These values include: -Services -Components -Helpers -Modifiers -Templates + +* Services +* Components +* Helpers +* Modifiers +* Templates + ember-strict-resolver defines a stricter and therefore much faster version of these lookups, which results in a 1:1 instead of 1:N resolution. In the flagship web app at LinkedIn, adopting ember-strict-resolver resulted in the following improvements (per a [TracerBench](https://www.tracerbench.com/) analysis): From 30ff403146c53f98e5f224184cd92b97ceb01413 Mon Sep 17 00:00:00 2001 From: Gabriel Csapo Date: Thu, 14 Jan 2021 10:02:34 -0800 Subject: [PATCH 6/6] Update text/0683-deprecate-fallback-resolution-ember-resolver.md Co-authored-by: Robert Jackson --- text/0683-deprecate-fallback-resolution-ember-resolver.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0683-deprecate-fallback-resolution-ember-resolver.md b/text/0683-deprecate-fallback-resolution-ember-resolver.md index 5f3a1e4bfc..a920aff4aa 100644 --- a/text/0683-deprecate-fallback-resolution-ember-resolver.md +++ b/text/0683-deprecate-fallback-resolution-ember-resolver.md @@ -83,8 +83,8 @@ Deprecating the loose-model in ember-resolver should happen in the next major ve ### Questions -can an app turn on the strict resolver before all of its addons have migrated? -can addons switch to using the strict resolver internally and still be consumed by an app using the classic resolver? +* can an app turn on the strict resolver before all of its addons have migrated? +* can addons switch to using the strict resolver internally and still be consumed by an app using the classic resolver? ## How we teach this