Rich-Harris / packd

Rollup as a service (with a little help from Browserify)
https://packd.now.sh
MIT License
261 stars 24 forks source link

apollo-client broken #2

Open stubailo opened 7 years ago

stubailo commented 7 years ago

https://packd.now.sh/bundle/apollo-client@1.0.0

I think this package works fine in Angular apps with Rollup.

Rich-Harris commented 7 years ago

Thanks. Looks like this is caused by rollup-plugin-node-resolve being overzealous. It's trying to import whatwg-fetch from transport/networkInterface.js, and that's failing because whatwg-fetch doesn't have a pkg.module.

This will presumably happen for any ES015 module that imports a non-ES2015 package. Best thing to do is probably to remove the skip option and associated behaviour from node-resolve altogether — I can't think of any good it's doing, to be honest.

Rich-Harris commented 7 years ago

I made a couple of changes to rollup-plugin-node-resolve that I thought would fix this (https://github.com/rollup/rollup-plugin-node-resolve/pull/90, https://github.com/rollup/rollup-plugin-node-resolve/issues/91), but it didn't. The difference now is that it's no longer our fault 😀 The culprit is this file, which is imported by this one and which isn't an ES2015 module.

I don't quite understand why it looks like that, when if I build it locally it looks like this:

export var version = 'local';

Does that mean anything to you?

stubailo commented 7 years ago

Yeah, this is something we use to try to make the version available, and is kinda broken anyway.

It's set in the publish script, I think it's deploy.sh in the repository.

Is that code not a valid ES2015 module?

Rich-Harris commented 7 years ago

Ah, right — this line is the culprit:

fs.writeFileSync('./npm/version.js', 'exports.version = \"' + package.version + '\"'); \

It needs to reflect the code that TypeScript generates (export var version = [pkg.version]) in order to work correctly. That shouldn't break your pkg.main, since it's a UMD (though that contains the hardcoded 'local' version).

I don't know if this helps in a TS workflow, but with rollup-plugin-json you can do this...

export { version } from './package.json';`

...and the rest of package.json will get tree-shaken. Maybe then you don't need that line in the deploy script at all?