Cannot Use External Module due to a call to require in dependency

[cthos]

Stencil version:

 @stencil/core@0.7.5

I'm submitting a:

[ ] bug report
[x] feature request
[ ] support request => Please do not submit support requests here, use one of these channels: https://forum.ionicframework.com/ or https://stencil-worldwide.slack.com

Current behavior:

Cannot use the cross-fetch module (or any other module that includes a require statement) in a stencil generated component because when loading the app at runtime, I'll get an error which is some variation of require is not defined, coming from an internal dependency.

I believe I need to be able to specify browser: true right around here in the rollup configuration - https://github.com/ionic-team/stencil/blob/master/src/compiler/bundle/rollup-bundle.ts#L27.

I'm not very well versed in rollup configuration (though another person helped me look into this and this was his conclusion), so I'm not 100% sure this is the proper solution but I can't modify the config enough to try.

Expected behavior:

Ideally I'd be able to specify that I want the bundled dependency code built for the browser, either through a config flag or just by making it the default.

Alternatively being able to override the whole of the rollup config in some manner would allow me to tinker with it.

Steps to reproduce:

I've created an app here which illustrates the problem:
https://github.com/cthos/stencil-rollup-repro-app

That app imports an illustrative library here - https://github.com/cthos/stencil-rollup-repro-sdk - that exports its dependencies as ES6 for Stencil which get picked up normally, but then at runtime you'll find the runtime error about require missing.

Related code:

// insert any relevant code here

Other information:

[bitflower]

I had rollup problems as well which were solved from 0.7.2 on... Maybe you try the latest version. It's always recommend. Stencil internals change very quickly

Ref #618

[cthos]

@bitflower - Yeah, good idea!

I can confirm that 0.7.5 produces the same error. (I've updated the main comment)

[jgw96]

@jthoms1 any ideas here? I can reproduce with the latest.

[bitflower]

Hi, in my first comment I referenced the wrong issue of mine.

I think this feature (or whatever will be the soltution) is related to this problem with the ‚debug‘ module: #494 I‘m having.

Stencil picks the node version of the module not the browser version.

With such a described flag and the repsective handling it could be solved.

[cdock1029]

Having same issue trying to use Apollo client.

I think this covers what I'm seeing, a dependency uses both require and import causing issues for commonjs plugin:

apollographql/apollo-link#558

rollup/rollup#1058 (comment)

[bitflower]

I have adapted my use case for the Stencil team in this repo: https://github.com/bitflower/caseos-feathers/tree/stencil-issue

It seems it's actually exactly the same use case then @cthos's: Writing a module in TS that exports with ES6 modules (for Stencil) but uses require (classic) commonjs modules (of course).

I have added you guys as collabs on this private repo. Let me know if you need anything else!

EDIT: Not sure if is not related to https://github.com/ionic-team/stencil/issues/686 as I'm npm linking during development

[arjunyel]

I'm also having an error with Stencil and Apollo, if you need another repo Apollo code is in src/components/my-app/my-app.tsx: https://github.com/arjunyel/stencil-apollo-error

[jthoms1]

@arjunyel You can add the following to your stencil.config.js file to get around the first issue.

  commonjs: {
    namedExports: {
      'node_modules/graphql-anywhere/lib/async.js': [ 'graphql' ]
    }
  }

But after this we run into the same issue, but looks like Apollo is fixing this in their code.

apollographql/apollo-link#558

[arjunyel]

Heres another repo if you need anymore, in general could this be considered a bug in the offending library? jasonkuhrt/graphql-request#74

[cthos]

So yeah, any idea on how to move forward on this?

I can't control upstream repositories and tell them to remove require, especially if they're not familiar with Typescript, or don't want to provide ES6 exports. Feels like we'd run into this fairly frequently when trying to use npm packages.

[jthoms1]

@cthos Can I assume that you are building an application with Stencil vs a collection of components? Just trying to collect some details. Also if you can provide packages where you have had the issue it would be helpful.

[cthos]

@jthoms1 - So the basic use case is I'm providing an SDK which provides a bunch of different targets (UMD, ES6 modules, CommonJS modules) - and it uses a Fetch polyfill so it'll work across node projects and browser projects.

The problem (which I've hopefully distilled to its essence in those example repos) is that if you try to use the ES6 modules in a Stencil project so you get the type hinting and TS importing goodness, you'll hit that error.

It could be gotten around in the end-app by binding the repo to a global and using the UMD bundle or something, but you lose the hinting and whatnot.

It'd probably be the same if you wanted a component you're exporting for use elsewhere to use that SDK internally. 🤔

So far I've only seen it in cross-fetch.

[jthoms1]

@cthos The more I dig in the more it seems like we should provide the ability to set the node resolve targets. Right now we are allowing module, jsnext, and main. For cross-fetch the issue is actually in node-fetch.

node-fetch has a package.json which says it supports es modules with the module tag but then uses requires in the actual code. Rollup does not like this.

https://unpkg.com/node-fetch@2.1.2/lib/index.es.js

I believe all of this could be avoided if we allow stencil users to also use browser as a node resolve target. I think that we could just expose this as an option in our config file.

[cthos]

I believe all of this could be avoided if we allow stencil users to also use browser as a node resolve target. I think that we could just expose this as an option in our config file.

Yeah, I think that'd work just fine! 👍

[jthoms1]

I verified that commit #760 does resolve this issue. Please update your config to the following. Note that the only change was adding nodeResolve with browser as true.

This is per the documentation provided by rollup-plugin-node-resolve. https://github.com/rollup/rollup-plugin-node-resolve#usage

exports.config = {
  nodeResolve: {
    browser: true
  }
};

exports.devServer = {
  root: 'www',
  watchGlob: '**/**'
};

[arjunyel]

Is there a downside to setting this as default for Stencil and/or the starter kits? Btw the patch fixed my issue, thank you :D

[jthoms1]

I was afraid this change could break existing bundles for other developers. If you set browser to true then browser becomes the preferred method and could change some builds.

This might change in the future where we always want to default to browser but I was being conservative when introducing this change.

[cthos]

If you set browser to true then browser becomes the preferred method and could change some builds.

I've not actually tried it, but does setting browser: true break SSR?

[jthoms1]

no, SSR uses jsdom and should not be broken by this change.