Main inside package.json is referencing to es6 modules

[isidrok]

Bug Main inside package.json is referencing to es6 modules

What MDC-Web Version are you using?

This happens with all components which have different versions, for instance the TextField is in version 0.3.1

What are the steps to reproduce the bug?

1. Create a project with npm init
2. Install the TextField component

npm i --save  @material/textfield

3. Configure rollup or webpack to bundle the project and set babel to exclude 'node_modules/**'
4. Somewhere in your project use the textfield:

import * as textfield from '@material/textfield';
mdcComponent = new textfield.MDCTextfield(element)

5. Check that the bundled app has the TextField component written in es6
6. Go to node_modules/@material/textfield
7. Change "main" from "index.js" to "dist/mdc.textfield.min.js"
8. Create the bundle again, note that this time it will not find the module
9. Change "main" back to "index.js"
10. Stop excluding 'node_modules/**' in the babel configuration
11. Should work fine this time

What is the expected behavior?

The "main" property inside the package.json should reference to the transpiled and bundled module instead of the es6 version so developers don't have to transpile it in order to avoid incompatibilities.
For example the TextField component should be pointing to dist/mdc.textfield.min.js instead of index.js.

There's actually another problem because the dist versions are exporting the utilities differently. The following snippet should work equally in the original and the minified/transpiled version:

import * as textfield from '@material/textfield';
mdcComponent = new textfield.MDCTextfield(element)

but using the later i get the following error:
'MDCTextfield' is not exported by 'node_modules@material\textfield\dist\mdc.textfield.min.js'

What is the actual behavior?

The "main" is pointing to index.js which contains the es6 module, and if I don't transpile it some things won't work, for example I cannot use Uglify.

(uglify plugin) Error transforming bundle with 'uglify' plugin: Unexpected token: name (MDCFoundation)

That's because it finds MDCFoundation defined as a class.

Any other information you believe would be useful?

There is a proposal to specify two entry points or main references inside package.json:

...
"main": "dist/app.min.js",
"jsnext:main": "index.js",
...

For more information refer to this issue: jsforum/jsforum#5

[Shyam-Chen]

[...]
  "module": "lib/es/index.js",  // for ES modules
  "main": "lib/cjs/index.js",  // for CommonJS modules
[...]

[brunoscopelliti]

I had this same problem several time.
When generate the build using webpack, I managed to work around configuring babel-loader so that it also transpiles files under node_modules/@material.

{
  test: /\.js$/,
  include: [
    appFolder,
    path.resolve(__dirname, "node_modules/@material")
  ],
  loader: "babel-loader"
}

This worked, but I didn't get very far, cause the same issue is currently beating me in my unit tests (where I don't use webpack, cause I don't like too much none of the suggested setup https://github.com/avajs/ava/blob/master/docs/recipes/precompiling-with-webpack.md).
I use ava for unit tests. ava default behaviour is of transpiling test files, but not the source files. In order to avoid issues with my own source file I configured ava so that it always include babel-register; from my package.json

  "ava": {
    "require": [
      "babel-register"
     ],
    "babel": "inherit"
  },

With this configuration my own source files get transpiled, but not node_modules, cause babel default is ignoring them.
I tried to change babel default behaviour by setting ignore, and only options in various form, both at project level, both specifically for ava, but I had no luck; finally discovered that probably there're bug affecting these options in babel 6.* (babel/babel#5872 (comment)).
Updating to babel 7.* alpha is not an option for me at this time...

I would like to know if you plan to modify your package.json as suggested above, or if you have any suggestions/work around for this case.

Thank you

[Shyam-Chen]

Temporarily use this.

import { __moduleExports as mdRipple } from '@material/ripple/dist/mdc.ripple';

[].forEach.call(
  document.querySelectorAll('.mdc-button'),
  ripple => mdRipple.MDCRipple.attachTo(ripple)
);

[brunoscopelliti]

That worked (almost)... since I was already transpiling my own sources, I had just to modify the import like this:

import { Ripple } from '@material/ripple/dist/mdc.ripple';

Still, looking forward for a better solution, that allows to remove the ugly import.
Thanks @Shyam-Chen

[isidrok]

For me the only option that works at the moment is the one @Shyam-Chen suggests, I made a list of possible imports and those are the results:

import imports from '@material/textfield/dist/mdc.textfield';
// which is equivalent to import {default as imports } from '@material/textfield/dist/mdc.textfield';
imports = undefined

import { MDCTextfield } from '@material/textfield/dist/mdc.textfield';
// Non-existent export 'MDCTextField' is imported from...

import * as imports from '@material/textfield/dist/mdc.textfield';
imports = {
	default: undefined,
	__moduleExports: {
		MDCTextfield,
		MDCTextfieldFoundation
	}
}

import * as imports  from '@material/textfield';
imports = {
    MDCTextfield,
    MDCTextfieldFoundation
}

Don't know why is webpack placing the exports under __moduleExports or if that is the normal behaviour though.

[walexnelson]

I can't tell you how long I spent trying to figure out all of the UglifyJS errors I was getting because of this. I'm running my JS through the babel-loader in webpack and have it excluding node_modules like I usually do. It took me a while to realize that the @Material modules need to be transpiled too if you import them like you would a typical module, at least if you want to import material-components-web rather than importing the individual transpiled @Material modules from their respective ./dist folders.

At the very least I think this needs to be documented unless I've been searching in all the wrong places.

I'm currently using a similar approach to @brunoscopelliti and it seems to be working. UglifyJS isn't choking on ES6+ syntax found in my bundle at least. Here's the babel-loader rule for my config so far. I'm using webpack for my tests so I haven't run into the same issues as @brunoscopelliti on that front.

{
  test: /\.js$/,
  include: [
    path.resolve(__dirname, './node_modules/@material'),
    path.resolve(__dirname, './src'),    
  ],
  use: [
    'cache-loader',
    'babel-loader'
  ]
},

[isidrok]

Well if someone is wondering how to solve this directly with Babel I use the include option (it cannot be used with exclude, everything not included will be excluded).

babel({
    include: [
       'node_modules/@material/**',
       'src/**'
    ]
})

[ciromattia]

Being bound to Laravel Mix I can't follow the babel/include way to include material-components-web into my js, after some tough headaches in the meantime I workarounded including the whole dists like

import * as mdc from 'material-components-web/dist/material-components-web';

Still following for a less hackish solution ;)

[tyronedougherty]

I'd just like to +1 my support recommending the change to the main in the package.jsons, so that they point at the dist folder instead. At the moment to solve it I'm having to reference the dist folder directly, for example:

import { MDCDialogFoundation } from '@material/dialog/dist/mdc.dialog';

It's a pretty common pattern across just about every other repository you can find on NPM. I'm not sure why it's not the case here. Some examples can be found in my duplicate issue.

[treybrisbane]

Also +1 on this.

By referencing ES6+ code in the main entry, you're effectively forcing your library consumers to have to either care about transpilation or go digging through the dist folder (as Tyrone mentioned above).

[chriscasola]

Is there any movement on this? I can open a PR if there is consensus on what should happen.

[isidrok]

In case someone is having problems with jest and babel-jest, exclude @material modules from transformation:

{
  "transform": {
    "^.+\\.jsx$": "babel-jest",
    "^.+\\.js$": "babel-jest"
  },
  "transformIgnorePatterns": [
    "node_modules/(?!@material)"
  ],
} 

[brneto]

I did this:

"jest": {
    "setupTestFrameworkScriptFile": "<rootDir>/src/setupTests.js",
    "transformIgnorePatterns": ["node_modules/(?!@material)"]
  }

But I'm still facing the following error:

import MDCTopAppBarAdapter from './adapter';
^^^^^^

    SyntaxError: Unexpected token import

      3 | import { bindActionCreators } from 'redux';
      4 | import { connect } from 'react-redux';
    > 5 | import { MDCTopAppBar } from '@material/top-app-bar';
        | ^
      6 | import * as actionCreators from '../../redux/actions';
      7 | import * as routerThunks from '../../redux/thunks/routerThunks';
      8 |

      at ScriptTransformer._transformAndBuildScript (node_modules/jest-runtime/build/script_transformer.js:402:17)
      at Object.<anonymous> (src/client/components/commons/NavBar.js:5:1)

Any advice about how would I to fix that?

[blowery]

+1 to getting this fixed. The general convention these days is to have package.json's main property point to commonJS exported es5 (with an optional UMD wrapper). You could also add a module property that points to es5 code exported using ES6 exports (but no es6+ code there).

There's really no current spec for exposing untranspiled es6+ code in a standard way. There's some movement around a jsmain:next property, but how to have clients reliably transpile the code is up in the air. See https://babeljs.io/blog/2018/06/26/on-consuming-and-publishing-es2015+-packages

In the current state, folks that want to use the core modules have to set up transpilation to use them, which opens the door to version mismatches and configuration mismatches and bugs.

[kfranqueiro]

We've changed our packages' main properties to point to dist resources in #3245. This will be available in 0.41.0.

Thanks everyone for the input and sorry for the delay. (Also thanks @moog16 for starting the ball rolling and doing some testing for the PR.)