Financial-Times / o-grid

Responsive grid system
http://registry.origami.ft.com/components/o-grid
93 stars 14 forks source link

Fix ambiguity of export default {} #177

Closed i-like-robots closed 5 years ago

i-like-robots commented 5 years ago

I'm assuming this is an error rather than by design, but I'm not sure...

What

The JavaScript entry point for this module has a default export of an object containing its API methods (export default { ... }). I've assumed that the intention of this was to provide the methods as named exports which can be imported individually, e.g.:

import { getCurrentLayout } from 'o-grid'

However this would currently require destructuring twice as shown below (please note that I've expanded the first line, the shorthand import x from 'y' is syntactical sugar for this):

import { default as oGrid } from 'o-grid'
const { getCurrentLayout } = oGrid

Inconsistent usage has been enabled by transpiling and bundling tools such as Babel and Webpack which supported module syntax loosely but are now moving towards closer spec compliance (see https://github.com/babel/babel/issues/2212#issuecomment-131110500 and https://babeljs.io/docs/en/v7-migration-api#export-changes)

Details

Assuming the intention was for these methods to be named exports I believe the source code should be updated to use the named exports syntax.

i-like-robots commented 5 years ago

It's used as a default export here:

And as a named export here:

So this would indicate sticking to using a default export for the current major version to maintain compatibility.