data icon indicating copy to clipboard operation
data copied to clipboard

[QUEST] ๐ŸŒฒProject Trim ๐ŸŒฒ

Open runspired opened this issue 6 years ago โ€ข 13 comments

๐ŸŒฒ Project Trim๐ŸŒฒ

As the Ember Community approaches delivery of long awaited features enabling tree-shaking, ember-data needs to take some steps to enable this feature to shed the weight of features that consuming applications do not use.

The first step is to ensure that the packages ember-data provides are properly encapsulated. In many cases, this will both enable support for tree-shaking once available to the broader community and enable manually opting for a subset of packages.

Later steps will ensure that ember-data relies on explicit imports (whether automatically generated at build-time or declared by consuming applications) where currently we rely on resolver lookups.


๐ŸŒฒ Use ember-data without @ember-data/serializer

The serializer package provides reference serializer implementations and a base serializer class that are not required for applications that choose to write a custom serializer implementing the minimal serializer interface. This includes the EmbeddedRecordsMixin and the base Transform class.

Transforms, while specified via attr available from @ember-data/model are a concept specific to these reference serializers and not used anywhere else within ember-data. Custom serializers thus may or may not choose to use them via the same mechanism (owner.lookup('transform:<name>')). Lastly the Transform base class merely shows the interface required for transforms but is not required to be used as a base class.

Action Items

  • [x] ๐Ÿ”’ ~#6113~ #6451 add a public Typescript interface for the minimal required serializer interface
  • [x] ๐Ÿ”’ #6086 eliminate registration of default transforms (use app re-exports instead)
  • [x] ๐Ÿ”’ #6086 eliminate injection of -default and -json-api serializer (use app re-exports instead)
  • [x] ๐Ÿ”’https://github.com/emberjs/rfcs/pull/522 deprecate -default serializer fallback in Store.serializerFor
  • [x] ๐Ÿ”’https://github.com/emberjs/rfcs/pull/522 deprecate Adapter.serializer and Adapter.defaultSerializer fallbacks (which currently default to -json-api.
  • [x] ๐Ÿ”’https://github.com/emberjs/rfcs/pull/522 deprecate Model.toJSON and support the scenario for quick debug-mode inspection via debugData() method which in DEBUG would print a similar object to the current toJSON method.
  • [x] implement serializer deprecations from https://github.com/emberjs/rfcs/pull/522
  • [x] ๐Ÿ”’ #6368 remove -rest serializer fallback usage in tests
  • [x] ๐Ÿ”’ #6369 #6370 remove -json-api serializer fallback usage in tests
  • [x] remove -default serializer fallback usage in tests
  • [x] add serializer encapsulation test
  • [x] (Side Quest) Add tests for the MinimumSerializerInterface (#6460 )

๐ŸŒฒ Use ember-data without @ember-data/adapter

The adapter package provides reference adapter implementations and a base adapter class that are not required for applications that choose to write a custom adapter implementing the minimal adapter interface.

Action Items

  • [x] add a public Typescript interface for the minimal required adapter interface
  • [x] eliminate injection of -rest and -json-api adapter (use re-export pattern for -json-api adapter)
  • [x] ๐Ÿ”’https://github.com/emberjs/rfcs/pull/522 deprecate Store.defaultAdapter (-json-api) and the -json-api adapter fallback behavior
  • [x] ๐Ÿ”’ #6353 eliminate -rest adapter which is only available for tests at this point unless consumers manually call adapterFor('-rest').
  • cleanup minimum required adapter interface
    • [x] findMany is optional
    • [x] findBelongsTo is optional
    • [x] findHasMany is optional
    • [x] shouldReloadRecord is optional
    • [x] shouldBackgroundReloadRecord is optional
    • [x] shouldReloadAll is optional
    • [x] shouldBackgroundReloadAll is optional
  • [x] implement adapter deprecations from https://github.com/emberjs/rfcs/pull/522
  • [x] test for isInvalidError flag or specific message instead of instanceof InvalidError in store
  • [x] ensure internal usage of errorsArrayToHash uses the private store version
  • [x] (Side Quest) Add tests for the MinimumAdapterInterface
  • [ ] move coalescing logic into the adapters from the store.

๐ŸŒฒ Use ember-data without @ember-data/model

While at the time of writing this quest @ember-data/model is the only model class available for use with ember-data based on public APIs, the introduction of custom Model Classes will mean that users who install @ember-data/model alternatives via addon or by writing their own should be able to consume ember-data without using a package which provides a model implementation they do not use.

Enabling use without @ember-data/model requires much more groundwork than that needed for the adapter and serializer package.

Model Action Items

  • [x] move the implementation of Model from the store package into the Model package.
  • [x] move the implementation of @hasMany and @belongsTo into the Model package.
  • [x] move -private/system/relationships/ext.js in @ember-data/store into the Model package
  • [x] move the implementation of ManyArray and PromiseManyArray into the Model package. See RFC notes
  • [x] move the implementation of PromiseBelongsTo into the Model package.
  • [x] inject the Model specific _modelForMixin implementation into the store from the Model package (this provides the multi-inheritance polymorphism feature which some aspects of ember-data currently support).
  • [x] deprecate BelongsToReference.push receiving a Model instance
  • [x] polish off the implementation of the Identifiers RFC.
    • [x] refactor conversion of Record to RecordIdentifier to occur within @belongsTo and @hasMany BLOCKED requires RecordData V2

InternalModel Action Items

  • [x] Deprecate/shim intimate Reference.internalModel property
  • [x] Refactor Store.getReference to not lookup the associated InternalModel to generate the reference.
  • [x] Move the state-machine into the Model package.
  • [x] Refactor store APIs to use identifiers instead of InternalModel
  • [x] Implement RecordData Errors RFC
  • [x] Implement Request State RFC.
  • [x] Move InternalModel into the Model package. (this is the LegacySupport class)

InternalModel Cleanup Action Items

Once the items in InternalModel Action Items are complete, we can move the instantiation and lookup logic in the Store into the Model package.

  • [x] create InternalModel only for @ember-data/model when using the instantiateRecord hook.
  • [x] Eliminate any remaining Store._internalModelForId usage in favor of Identifier based logic.

๐ŸŒฒ Use ember-data without @ember-data/relationship-layer

The relationship-layer package is currently deeply entangled with @ember-data/model's Model implementation. Even though we can encapsulate the imports to/from these packages, users would not be able to use @ember-data/model without also using @ember-data/relationship-layer. Additionally, in its current form the relationship-layer is not abstracted away from the specifics of the implementation of Model making is largely unusable with other Model implementations. Changes to address this are needed but are outside the scope of this quest, and until such time as they occur users wishing to drop this package would need to drop both this and the Model package together.

  • [x] move everything within -private/system/relationships/state in @ember-data/store into a new relationship-layer package. See RFC notes
  • [x] Move belongsToReference and HasManyReference into relationship-layer from store.
  • [x] refactor usage of internalModel to pull information from the RecordIdentifier or the Store for type id hasRecord and getRecord()
  • [x] isolate relationship fetching logic (currently spread across dozens of private store methods) ~into a private middleware to be used by the private -request-manager service~ (moved into the model layer to make it easier to write alternative relationship fetching logic for a new relationship layer)

๐ŸŒฒ Use ember-data without @ember-data/debug

Provides the DataAdapter for the ember-inspector

  • [x] refactor to use service injection, kill initializer injection
  • [x] refactor to manage own registration
  • [x] detect should not require Model
  • [x] stop automatically shipping to production (ship by config only)
  • [x] Add documentation for shipping the inspector in production if desired
  • [x] Add Encapsulation Test

๐ŸŒฒ Use ember-data without @ember-data/record-data

Provides the default RecordData implementation

  • [x] move RecordData into this package
  • [x] ~(stretch goal)~ Land RecordDataV2 Note: blocker on a key bit of model refactoring, no longer stretch goal
  • [x] ~(stretch goal)~ Public Docs for RecordData
  • [x] hook into store package to implement createRecordDataFor
  • [x] Add Encapsulation Test
    • [-] ~~(stretch goal) Move RecordData interface tests into the encapsulation test app~~

Misc

  • [x] ๐Ÿ”’ #6357 kill createStore and setupStore
    • #6353 allow incremental removal of these helpers
    • #6356 make removal easier to review
    • #6361 remove some usage of setupStore
    • #6347 remove all createStore usage
    • #6221 remove some usage
    • #6386 remove createStore
  • [x] ๐Ÿ”’ #6353 deprecate injection of -default serializer if moduleFor style tests are still supported
    • [x] ๐Ÿ”’ #6353 lazily register and the -default serializer when this condition is detected

๐ŸŒฒ Tree Shaking Strategy

embroider is pursuing a strategy in which consuming applications opt in to enabling tree-shaking of specific kinds of features once they have determined for their own application that this is allowable. For certain kinds of things embroider provides an escape hatch to not-shake. Currently this includes models, adapters, and serializers and their usage is not easy to statically analyze. Here we outline our plan for reducing the costs of these primitives, but these are not a requirement to complete ProjectTrim.

Models - Users will be able to opt-in to a new lighter-weight model experience

This model experience would be heavily informed by ember-m3 (which configures ember-data via public APIs to use its own model). Goals would be for model files to be optional, and to make their usage tree-shakeable.

Adapters/Serializers - Once the deprecation of automatic adapter fallbacks is resolved, we could find a way to let embroider know that the adapters in @ember-data/adapter are shakeable if no user adapter imports them. However, we primarily recommend dropping the adapter package entirely in favor of authoring an adapter specific to your application's need. Further design work here is needed but going forward the plan is to introduce a network-layer that allows registration ofuser-supplied network middlewares. Such middlewares, including those provided by addons, would be imported and registered with the request-manager service directly. Since these are static imports being registered to a service, in the long term no custom infrastructure would be needed to support tree-shaking. A legacy middleware would delegate request management to existing adapters/serializers, removal of which would allow for full tree-shaking.

Also in the long-term, serializers would be deprecated in favor of either their corresponding "adapter" replacement handling these concerns or in favor of middlewares specific to normalization and serialization concerns. Similarly, in the long term no custom infra for ensuring static imports for tree-shaking needs would be required.

Transforms - There are three ways for a user to utilize a transform today: attr(<transformName>), owner.lookup('transform:<transformName>') and direct import of a transform followed by Transform.create(). Of these, the third already plays nicely with tree-shaking. For attr and lookup usage we will need to enforce "static string usage" for lookup and and attr(), at which point we can use this to generate an import file of only those transforms that are actually in-use. Alternatively, we may decide to deprecate transforms altogether in favor of transformations which do not require a container lookup.

Also note, if none of the default serializers is in use and there is no project usage of lookup('transform:<transformName>') then it is likely we can drop all transforms even when referenced by attr.

Note that using the adapter/serializer/record-data/model/debug packages is entirely optional already. If you are not using code from these packages and/or would prefer your own implementation of the public-interface they implement then installing @ember-data/store and whatever other @ember-data/ packages you require directly instead of installing ember-data will result in both the code those packages bring not being present and any code the store contains for configuring them to be removed at build-time.


๐ŸŒฒ Tree Shake all of ember-data

  • [-] ~deprecate defaultStore behavior in Ember~ no longer required due to injections having been deprecated and removed, but is a nice-to-have.
  • [x] deprecate injected store on controller/route

runspired avatar Jun 18 '19 01:06 runspired

kinda codemod draft https://astexplorer.net/#/gist/5a96ed2a14b06f69499c124fc7cf953f/0b9b195ca734c19c04435383008a30a621c4b529

lifeart avatar Jul 10 '19 22:07 lifeart

@lifeart hate to tell you this but we've got the codemod and lint rules mostly ready to go already.

rfc-data: https://github.com/ember-data/ember-data-rfc395-data codemod: https://github.com/dcyriller/ember-data-codemod

I think we still need to update the lint plugin to pull from this rfc-data

runspired avatar Jul 10 '19 23:07 runspired

@runspired good news! @dcyriller should we move codemod into https://github.com/ember-codemods ? //cc @rwjblue // // #topic-codemods

lifeart avatar Jul 11 '19 08:07 lifeart

I created an issue to discuss that over in the codemod repo...

rwjblue avatar Jul 11 '19 14:07 rwjblue

@lifeart good idea, sure I'll do that. Let me fix a bug and initiate the migration.

dcyriller avatar Jul 11 '19 15:07 dcyriller

There is node a lint rule PR here: https://github.com/ember-cli/eslint-plugin-ember/pull/450

runspired avatar Jul 11 '19 18:07 runspired

Is this up to date? It looks like there has been little progress since December 2019.

Gaurav0 avatar May 28 '20 18:05 Gaurav0

Is this up to date?

ryanolsonx avatar Sep 16 '21 20:09 ryanolsonx

@ryanolsonx yes it is :)

runspired avatar Apr 15 '22 07:04 runspired

#8078 once merged will complete all internalModel tasks, so I've checked them off. This leaves primarily the RecordData v2 tasks (and request coalescing).

runspired avatar Jul 26 '22 23:07 runspired

#8084 picked up where #8078 left off and finally slays the beast. Polymorphic Relationships RFC (kill mixins!) and RecordData V2 are the next priorities.

runspired avatar Aug 05 '22 06:08 runspired

Once #8122 lands we will have only one last lonely task to complete this epic! It's a tricky one, but its also a key bit of code re-shuffling as we prepare to introduce the replacement for adapter/serializer.

With Trim complete, the focus will primarily shift to the items in the roadmap for 5.0 #8086. However, there's been a long running epic (without a publicly attached issue) to "unplug" from Ember itself, where to "unplug" means that we are no longer dependent on any framework code and could be used by the broader JS community as desired. The 5.0 Roadmap accomplishes the major steps needed for this, but there are tons of things not on that roadmap we could use help with. If folks are interested I can whip up a tracking ticket.

runspired avatar Aug 14 '22 11:08 runspired

Awesome work runspired! We've doubled the sponsorship to "Extra Appreciative Of The Work" now. Looking forward to 5.0!

If folks are interested (see runspired's question above), we are willing to sponsor contributions on Ember Data (we're already sponsoring 3 people contributing to Ember Data).

sandstrom avatar Aug 14 '22 14:08 sandstrom

At long last :wave:

runspired avatar Mar 26 '23 23:03 runspired