Automatically expose all ES6 imports in core modules

[chillu]

The framework/Gulpfile.js exposes a lot of ES6 modules (both vendor and custom code). This is used to avoid duplicating code in bundle files in other modules. For example, the JS code for the react ES6 module is only included once in framework, and then referenced in asset-admin/Gulpfile.js.

This approach leads to fragile bundles: If a developer forgets to expose an ES6 module in framework , it'll be inaccessible to other SilverStripe modules. If other SilverStripe modules pull in NPM dependencies for those ES6 modules as well, this can lead to the same JavaScript code being bundled twice. This works, due to the closure isolation in individual bundle files. But it makes it very hard to debug and set breakpoints.

framework/Gulpfile.js

.require(`${PATHS.MODULES}/bootstrap/dist/js/umd/collapse.js`,
  { expose: 'bootstrap-collapse' }
)
.require(`${PATHS.ADMIN_JS_SRC}/components/Form/Form`,
  { expose: 'components/Form/Form' }
)
.require(`${PATHS.ADMIN_JS_SRC}/components/Form/FormConstants`,
  { expose: 'components/Form/FormConstants' }
)
.require(`${PATHS.ADMIN_JS_SRC}/components/FormAction/FormAction`,
  { expose: 'components/FormAction/FormAction' }
)
.require(`${PATHS.ADMIN_JS_SRC}/components/FormBuilder/FormBuilder`,
  { expose: 'components/FormBuilder/FormBuilder' }
)
.require(`${PATHS.ADMIN_JS_SRC}/components/GridField/GridField`,
  { expose: 'components/GridField/GridField' }
)
...

asset-admin/Gulpfile.js

.external('components/TextField/TextField')
.external('components/FormAction/FormAction')
.external('deep-freeze')
.external('lib/Config')
.external('react')
.external('jQuery')
.external('i18n')
.external('lib/SilverStripeComponent')
.external('lib/Backend')
.external('react-dom')
.external('react-addons-test-utils')
...

Options:

• Create a browserify plugin which automatically exposes all modules it loads through require/import
• Publish SilverStripe's custom ES6 code in framework as an NPM repo, add it to other SilverStripe modules as a dependency - and live with the bundle code duplication and potential version mismatches. This would still require expose on react and other "heavy" dependencies, we can't duplicate those in each bundle. Would also fix the current NPM include path issues (IDEs don't pick up import components/Form/Form in asset-admin since it's not defined in node_modules).

[sminnee]

This has been rendered obsolete by Webpack. Any further comments, @chillu ?

[chillu]

Hmmm, I believe it's the same issue, just in a different build tool: We still have lots of expose and externals calls - presumably they'll exhibit the same behaviour if you forget to add one (bundling duplicates)

https://github.com/silverstripe/silverstripe-framework/blob/master/admin/client/src/bundles/vendor.js#L14
https://github.com/silverstripe/silverstripe-framework/blob/master/webpack.config.js#L60

[chillu]

The CommonsChunkPlugin will ensure that there's no duplication between framework/admin/client/dist/js/vendor.js and framework/admin/client/dist/js/bundle.js within framework: https://github.com/silverstripe/silverstripe-framework/blob/master/webpack.config.js#L135. But there's nothing warning you about bundling duplicates in asset-admin/client/dist/js/bundle.js

[chillu]

Replaced by silverstripe/webpack-config#1