Skip to content
This repository has been archived by the owner on Aug 4, 2021. It is now read-only.

Use named exports as a function when no default export is defined #44

Closed
wants to merge 1 commit into from
Closed

Use named exports as a function when no default export is defined #44

wants to merge 1 commit into from

Conversation

dralletje
Copy link
Contributor

Fixes rollup/rollup#524

Will allow commonjs modules to import es modules when no default is defined. A lot of modules still rely on the nodejs way: The ability to import an object of exports when using require. Rollup didn’t allow that, until now :)

@Victorystick
Copy link
Contributor

It looks good to me. It adds a bit of complexity, but I've encountered scenarios where this is absolutely necessary if Rollup is to bundle the code. 👍 cc @rollup/collaborators

In time, we may want to refactor the 'default' in name ? ... bit to be like the export code. Perhaps the CommonJS plugin could consolidate its implementation with Rollup's _interopRequire?

@TrySound
Copy link
Member

@Victorystick LGTM. I'd like to release this feature, caz I can't build bundle with react-redux. :)

@Victorystick
Copy link
Contributor

@TrySound Go ahead.

@TrySound
Copy link
Member

TrySound commented Mar 1, 2016

@Victorystick I don't have access for this repo :)

@Victorystick
Copy link
Contributor

@TrySound Oops. How about now?

@TrySound
Copy link
Member

TrySound commented Mar 2, 2016

@Victorystick Yep. Thanks.

@dralletje
Copy link
Contributor Author

@Victorystick I can't seem to find any _interopRequire in Rollup, could you point me in the right direction? 😅

@Victorystick
Copy link
Contributor

@dralletje Sorry, I meant _interopDefault. It's currently only used here, but should probably be used by all finalisers.

@chadly
Copy link

chadly commented Mar 3, 2016

👍 please. I, also, am trying to bundle with react-redux

@dtothefp
Copy link

dtothefp commented Mar 9, 2016

@dralletje this looks like it almost addresses an issue I have with code that has been pre-compiled with babel having import/require ['default'] resulting in undefined values. I found this issue with parse

https://github.com/ParsePlatform/Parse-SDK-JS/blob/master/src/Parse.js

The compiled code will result in

var _interopRequireDefault = require('babel-runtime/helpers/interop-require-default')['default'];
var _interopRequireWildcard = require('babel-runtime/helpers/interop-require-wildcard')['default'];

and with my config

const babelrc = { 
  babelrc: false,
  runtimeHelpers: true,
  presets: ['es2015-rollup', 'stage-0'],
  plugins: ['transform-decorators-legacy'] 
};
const src = path.join(srcDir, scriptDir);
const srcGlob = path.join(src, '{*.js,**/*.js}');
const hfaGlob = 'node_modules/@hfa/{js-api-wrappers,validator}/lib/{*.js,**/*.js}';

rollup({
  entry,
  plugins: [
    babel({
      ...babelrc,
      sourceMaps: true,
      include: [
        srcGlob,
        hfaGlob
      ]
    }),
    nodeResolve({
      extensions: ['.js'],
      main: true,
      browser: true,
      preferBuiltins: false
    }),
    commonjs({
      include: [
        'node_modules/**/*.js'
      ],
      exclude: [
        ...hfaGlobs
      ]
    })
  ]
}).then((bundle) => {

without your fix will result in

var require$$9$1 = (createClass && typeof createClass === 'object' && 'default' in createClass ? createClass['default'] : createClass);
var _createClass = require$$9$1['default'];
var _classCallCheck = require$$2$28['default'];

resulting in

Uncaught TypeError: _createClass is not a function

with your branch I get

var createClass$1 = (createClass && typeof createClass === 'object' && 'default' in createClass ? createClass['default'] : createClass);

var require$$9$1 = Object.freeze({
     default: createClass$1
});
var _createClass = ('default' in require$$9$1 ? require$$9$1['default'] : require$$9$1)['default'];
var _classCallCheck = ('default' in require$$8$2 ? require$$8$2['default'] : require$$8$2)['default'];

which still throws the same error but looks a lot closer to working, except for the trailing ['default'] after the ternary. I'm not sure if this is what @Victorystick but if so and this problem could be resolved it would potentially resolve #29. Any feedback on when/if this will be merged and if it could potentially address #29?

@lukasjuhas
Copy link

Uhh any updates regarding this? I'm stuck here. Thanks!

@JRJurman
Copy link

What's the status of this PR? Is there an alternative (specifically for including react-redux), or should this be merged in?

@TrySound
Copy link
Member

@dralletje Can you rebase PR and I will merge it.

@raphaelokon
Copy link

raphaelokon commented Jul 15, 2016

@TrySound @dralletje I can do a fork and implement this feature if that is ok. I cannot run react-redux without it. Give me a ping and I will do a fork and implement it.

@TrySound
Copy link
Member

Would be great.

@raphaelokon
Copy link

@dralletje Is that okay for you?

@raphaelokon
Copy link

Okay, made progress. Protobuf test is failing for me. I will try to sort that out over the weekend.

@TrySound
Copy link
Member

@raphaelokon Heh, look at my PR :)

@raphaelokon
Copy link

I did now :-) I was traveling yesterday and just got the chance to tackle it. Anyways was good to look into the internals.

@TrySound
Copy link
Member

Protobuf failed without full interop checking.

@dralletje
Copy link
Contributor Author

Hey @raphaelokon , ofcourse that is okay :-) I hope you can use the logic in this PR so you don't have to figure out everything yourself again!

@raphaelokon
Copy link

@dralletje Cheers on that. @TrySound already did a PR that incorporates your stuff. Probably this and the defaultInterop PR can be closed then?

@TrySound
Copy link
Member

@raphaelokon Let's wait until Rich will publish it tomorrow.

@TrySound
Copy link
Member

Did you try to build it with copy pasta build of the last release?

@raphaelokon
Copy link

@TrySound Sorry, not sure I follow. I just implemented the feature for non-default exports on top of the current version. All tests (except the protobuf) were passing.

@TrySound
Copy link
Member

I meant your react redux project. Is it work with the latest changes. You may pull master, build code and copy paste to your node_modules.

@raphaelokon
Copy link

Ah … gotcha :> Will have to check tomorrow, but will defo let you know here.

@TrySound
Copy link
Member

Published 3.2.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants