data icon indicating copy to clipboard operation
data copied to clipboard

[FEAT] Rollup `-private` as part of prepublish

Open runspired opened this issue 5 years ago • 3 comments

EmberData usesRollup to merge all of our -private directories into single modules. -private represents a single module organized as many modules for maintenance ease. This benefits us in three ways:

  • It reduces the scope of potential private API's and modules being leaked for use by consuming applications
  • This greatly improved our parse time by having a single module instead of many modules (mostly via a reduction in the number of scopes and closures involved).
  • It reduces our bundle size significantly (some dead-code-elimination but mostly via the more advanced minification that a single module allows and elimination of lots of module declarations that would otherwise be present)

It also comes with a few drawbacks:

  • consuming applications pay the cost of the rollup step (as much as 3-5seconds)
  • embroider has to let EmberData do a double pass in order to receive our addon output

We should make all of the packages rollup their -private directories as a prepublish step such that consuming applications get a single -private module when they install one of our packages and don't pay the cost of rollup at all.

This will also make things play nicer with embroider because embroider will not need to run our build step before being able to construct the graph correctly, as our modules will be in the correct form by the time embroider sees them in consuming applications.

The rollup output should be an ES Module with as minimal transpilation done as possible so that we can respect the target of consuming applications. Getting the babel config right such that EmberData's needs and consuming app targets are both respected will likely be the more difficult bit here.

runspired avatar Apr 20 '19 23:04 runspired

Not sure if this is related, but with [email protected] we're currently seeing:

'@ember-data/store/-private' is imported by ../../../../tmp/broccoli-13026pikMS34IzFf3/cache-274-rollup/build/-private/index.js, but could not be resolved – treating it as an external dependency
'@ember-data/store' is imported by ../../../../tmp/broccoli-13026pikMS34IzFf3/cache-274-rollup/build/-private/index.js, but could not be resolved – treating it as an external dependency
'@ember-data/model' is imported by ../../../../tmp/broccoli-13026pikMS34IzFf3/cache-274-rollup/build/-private/system/debug/debug-adapter.js, but could not be resolved – treating it as an external dependency

richard-viney avatar May 31 '19 05:05 richard-viney

#6180 resolves the warnings and restores rollup but we still need to do the pre-publish

runspired avatar Jun 22 '19 01:06 runspired

@dcyriller is going to look into this as part of better preparing ember-data for embroider

runspired avatar May 28 '21 20:05 runspired

closing in favor of #8103 tracking this, @ember-data/tracking is an example of a library now doing this. #8285 is another example.

runspired avatar Nov 17 '22 03:11 runspired