Razzle: Issues with node externals

[therealgilles]

❓Question

I'm using the simple-oauth2 node module on the server side. It uses the (new) [private] class field syntax. Webpack tells me that it cannot parse the associated files as follows:

throw new Error("Module parse failed: Unexpected character '#' (19:2)\nYou may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders\n| \n| module.exports = class Client {\n>   #config = null;\n|   #client = null;\n| ");
      ^
Module parse failed: Unexpected character '#' (19:2)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
| 
| module.exports = class Client {
>   #config = null;
|   #client = null;
|
    at Object.../server/node_modules/simple-oauth2/lib/client/client.js (.../app/build/server.js:507057:7)
    at __webpack_require__ (,,,/app/webpack/bootstrap:754:1)

Is that something that's supposed to be supported?
If so, where am I going wrong in my config? babel/webpack?
I tried razzle@canary but no luck either.

Thanks for any insight and advice :)

[fivethreeo]

In canary you can use modifyBabelPreset and add https://babeljs.io/docs/en/babel-plugin-proposal-private-methods to Babel plugins

[fivethreeo]

Or simpler, add that to plugins in .babelrc

[therealgilles]

@fivethreeo, thanks for the suggestion. I tried adding it to .babelrc but the result was the same.

[therealgilles]

I realize now I should (1) have filed this under Discussions, and (2) explain my repo structure a bit more. So here we go:

I'm trying to add razzle to an existing create-react-app repo using redux. My repo structure looks like this:

|-- node_modules
|-- app
     |-- node_modules
     |-- src
     |-- index.js (client)
|-- server
     |-- node_modules
          |-- simple-oauth2
     |-- src
     |-- index.mjs (http server)
     |-- expressServer.mjs (express server)
|-- config
     |-- config.mjs (server config file)

I added razzle (and its dependencies) directly under my-app and then configure the path using razzle.config.js.

// razzle.config.js

const path = require('path')

module.exports = {
  options: {
    verbose: true,
  },

  modifyPaths({
    webpackObject,
    options: {
      razzleOptions,
    },
    paths,
  }) {
    paths.appSrc = path.join(paths.appPath, 'app/src')
    paths.appServerJs = path.join(paths.appPath, 'server/src/expressServer')
    paths.appServerIndexJs = path.join(paths.appPath, 'server/src/index')
    paths.appClientIndexJs = path.join(paths.appPath, 'app/src/index')

    return paths
  },
}

When I run 'npm start', I get the error I started this thread with or the one below:

my-app/build/server.js:3606
throw new Error("Module parse failed: Unexpected token (399:4)\nYou may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders\n|   const store = getStore({ server: true })\n|   const Application = (\n>     <Provider store={store}>\n|       <StaticRouter context={context} location={req.url}>\n|         <div>");
      ^

Module parse failed: Unexpected token (399:4)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
|   const store = getStore({ server: true })
|   const Application = (
>     <Provider store={store}>
|       <StaticRouter context={context} location={req.url}>
|         <div>

Here it's stumbling on the JSX syntax inside the server/src/expressServer.mjs file. I must be missing something completely obvious to someone more experienced. I will take any suggestion :)

[therealgilles]

It seems to be a path issue. If I create a brand new app with create-razzle-app and move the server.js file to a new server/src directory and change its location through appServerIndexJs in razzle.config.js, I get a similar error.

I'm guessing there must be a way to make this work though.

[therealgilles]

I made a bit of progress adding a jsconfig.json file with the following:

{
  "compilerOptions": {
    "baseUrl": "node_modules",
    "paths": {
      "AppSrc": ["../app/src"],
      "ServerSrc": ["../server/src"],
      "Config": ["../config"]
    }
  }
}

Now I'm back to the original error at the top of this thread.

[therealgilles]

It appears the issue is that simple-oauth2 is under my-app/server/node_modules. With a new create-razzle-app, if I put simple-oauth2 under my-app/node_modules, it works. If I move it under the server/node_modules, I get the error. It must have to do with the way babel or webpack work.

[fivethreeo]

You could try setting NODE_PATH=./server/node_modules

[therealgilles]

Thanks again for the suggestion. I ran NODE_PATH=./server/node_modules npm start and got the same error.

I get that because I'm trying to do SSR, the client and the server are now linked together and so they should really share a single package.json and node_modules directory, to avoid redundancy and version mismatches. Eventually I will probably do that.

But I'm curious as to why it's not working with the current directory structure and how to make it work. Seems like it has to do with how webpack decides to parse/process modules.

[fivethreeo]

Or NODE_PATH=../server/node_modules , I am working on a monorepo example, so will test this out

[therealgilles]

Same result, no go. I thought it had to do with the resolve.modules paths too, so I even tried to add a new paths.extraModules option to add paths to it, but that did not help either.

[therealgilles]

Traced the issue to this code in razzle/config/createConfigAsync.js:

      // Same as above: if the package, when required from the root,
      // would be different from what the real resolution would use, we
      // cannot externalize it.
      if (baseRes !== res) {
        return callback();
      }

res and baseRes do not match:

nodeExternalsFunc simple-oauth2
... context my-app-canary/server/src
... res my-app-canary/server/node_modules/simple-oauth2/index.js
... baseRes my-app-canary/node_modules/simple-oauth2/index.js

so it's not considering the file as external.

[fivethreeo]

Maybe we somehow should check all module dirs we add .. but how ..

[fivethreeo]

And how woud that work after a deploy when that may differ

[fivethreeo]

Or resolve that module relative to appdir in a webpack alias?

[therealgilles]

I need to educate myself about webpack externals. I stumbled about this stackoverflow article that talks about a library called webpack-node-externals. Not sure if it would be applicable here.

I haven't tried to deploy anything with razzle yet, so I'm not sure about that part.

Or resolve that module relative to appdir in a webpack alias?
Does it seem feasible to define a webpack alias for every single library though?

A good question at this point maybe whether it is a structure you would want to support.

[fivethreeo]

Razzle used that before, but it was a bit too unreliable. In canary this was added. Next.js also resolves externals like this.

[therealgilles]

How about a paths option to add webpack externals? I really don't know enough about razzle to make the right call here.

[fivethreeo]

https://github.com/vercel/next.js/blob/canary/packages/next/build/webpack-config.ts#L568

I think you can add more externals before or after this function to override specific modules.

[fivethreeo]

https://webpack.js.org/configuration/externals/#combining-syntaxes

[fivethreeo]

A complex issue, I have been wringing my brain on this one already ;)

[therealgilles]

Oh okay! And I see two separate issues here:
1) The webpack external may allow to skip trying to bundle the library.
2) But what if we actually want to transpile and bundle the library? will webpack 5 help support the syntax?
I thought that the babel plugin above could help with webpack 4 but that did not seem to work either.

[fivethreeo]

https://github.com/jaredpalmer/razzle/blob/master/packages/razzle/config/createConfigAsync.js#L495 - or reorder these

[fivethreeo]

webpack 5 or webpack 4 should not matteer

[fivethreeo]

I am thinking of adding a notExternals that is a list of regexes.

[therealgilles]

I mentioned webpack 5 earlier because even if babel does not transpile this code, I thought webpack 5 would have support for this syntax and so would not throw an error in the generated code.

I'm still a little fuzzy on how this all works. The current behavior seems to be not to transpile the code under my-app/node_modules, at least for the server compile, but what if I wanted/needed the code to be transpiled. Is there a way to achieve that?

[therealgilles]

I was able to make my create-razzle-app example work with either:

{ 
  "compilerOptions": {
    "baseUrl": "node_modules",
    "paths": {
      "Client": ["../src"],
      "Server": ["../server/src"],
      "ServerNodeModules": ["../server/node_modules"]
    }
  }
}

or:

{ 
  "compilerOptions": {
    "baseUrl": "node_modules",
    "paths": {
      "Client": ["../src"],
      "Server": ["../server/src"],
      "SimpleOAuth2": ["../server/node_modules/simple-oauth2"]
    }
  }
}

[fivethreeo]

The issue is that node js and browsers do not support this syntax. So babel has to transpile this code no matter what. I will try fixing this in canary tomorrow. I will add a new razzle option. notNodeExternals you can set to [/server/simple-oauth2/] also I should write some docs on this and serverless and maybe add serverLessNodeExternals, and clientExternals.

Someone wanted a library output aswell that could be used to make externals to be consumed in/outside webpack. But that will be 4.1 at the earliest.

[therealgilles]

I used to run node as follows for the server:
node --trace-warnings --experimental-modules --es-module-specifier-resolution=node

This supports class fields but I understand it's experimental. Which leads me to another request or suggestion. It would be great if it were possible to specify command-line options for node. Or is that already possible by overriding the NODE env variable?

[fivethreeo]

That works too :) But it should be nicer to do 😀

[fivethreeo]

I could add some razzle option to modify this https://github.com/jaredpalmer/razzle/blob/canary/packages/razzle/config/createConfigAsync.js#L615

[fivethreeo]

But when starting in production add those in package.json scripts.

[therealgilles]

Side question: is it right that 'server.js' is hardcoded on this line?

https://github.com/jaredpalmer/razzle/blob/0c2532c037d80bebf17c1e00123103620d5ce983/packages/razzle/config/createConfigAsync.js#L632

[fivethreeo]

Name is actually not used in that plugin. But entryName is hardcoded.

[therealgilles]

Hmm trying to run the create-razzle-app I was finally able to compile gave me this error:

Invariant failed: You should not use <Switch> outside a <Router>
    at invariant (.../my-app-canary/node_modules/tiny-invariant/dist/tiny-invariant.cjs.js:13:11)
    at Object.children (.../my-app-canary/node_modules/react-router/modules/Switch.js:17:11)
    at ReactDOMServerRenderer.render (.../my-app-canary/server/node_modules/react-dom/cjs/react-dom-server.node.development.js:3635:1)
    at ReactDOMServerRenderer.read (.../my-app-canary/build/webpack:/server/node_modules/react-dom/cjs/react-dom-server.node.development.js:3373:1)
    at renderToString (.../my-app-canary/build/webpack:/server/node_modules/react-dom/cjs/react-dom-server.node.development.js:3988:1)
    at .../my-app-canary/build/webpack:/server/src/server.js:42:1
    at Layer.handle [as handle_request] (.../my-app-canary/build/webpack:/server/node_modules/express/lib/router/layer.js:95:1)
    at next (.../my-app-canary/build/webpack:/server/node_modules/express/lib/router/route.js:137:1)
    at Route.dispatch (.../my-app-canary/build/webpack:/server/node_modules/express/lib/router/route.js:112:1)
    at Layer.handle [as handle_request] (.../my-app-canary/build/webpack:/server/node_modules/express/lib/router/layer.js:95:1)

so I guess I claimed victory too soon.

[fivethreeo]

Default example?

[fivethreeo]

Could be conflicting react-router-dom modules. Try using resolutions in package.json

[therealgilles]

Default example with server.js and index.js moved to server/src, and package.json (and associated node_modules) under server/. If I delete the server/node_modules directory, it works again. Maybe I am trying too hard to make my life more difficult.

Will try your suggestion.

[therealgilles]

Okay, I found a solution. Webpack needs to know which modules have priority, otherwise it will package all of them and get confused (apparently). I added this to razzle.config.js:

// razzle.config.js

const path = require('path')

module.exports = {
  options: {
    verbose: true,
  },

  modifyWebpackConfig({
    env: {
      target,
      dev,
    },
    webpackConfig,
    webpackObject,
    options,
    paths,
  }) {
    if (target === 'node') {
      // Tell webpack which modules have priority in case of duplicates
      webpackConfig.resolve.modules = [
        path.resolve(__dirname, 'node_modules'),
        path.resolve(__dirname, 'server/node_modules'),
      ];
    }

    return webpackConfig
  },

  modifyPaths({
    webpackObject,
    options,
    paths,
  }) {
    // Update location of server files
    paths.appServerIndexJs = path.join(paths.appPath, 'server/src/index')
    paths.appServerJs = path.join(paths.appPath, 'server/src/server')

    return paths
  },
}

[therealgilles]

Now trying to use that and fix the issue with externals (for simple-oauth2).

[therealgilles]

I'm thinking your notExternals option will do (until webpack supports all this syntax natively). In the meantime, I can add this to razzle.config.js under modifyWebpackConfig :

const resolveRequest = require('razzle-dev-utils/resolveRequest');

module.exports = {
  ...
  modifyWebpackConfig({
    ...
  }) {
    if (target === 'node') {
      webpackConfig.externals.unshift((context, request, callback) => {
        let res
        try {
          res = resolveRequest(request, `${context}/`)
        } catch (err) {
          return callback()
        }
        if (res.match(/simple-oauth2/) || (res.match(/node_modules/) && !res.match(/\/server\//))) {
          const externalRequest = path.posix.join(
            paths.appPath,
            path
            .relative(paths.appPath, res)
            // Windows path normalization
            .replace(/\\/g, '/')
          )
          return callback(undefined, `commonjs ${externalRequest}`)
        }
        return callback()
      })
    }
    ...
  }
}

[therealgilles]

I could alternatively add the simple-oauth2 path as alias in jsconfig.json:

{ 
  "compilerOptions": {
    "baseUrl": "node_modules",
    "paths": {
      "ServerSrc": ["../server/src"],
      "OAuth2": ["../server/node_modules/simple-oauth2"]
    }
  }
}

but then its module dependencies still get processed by webpack and create a much bigger bundle.

[therealgilles]

Wait, isn't razzleOptions.notExternalModules the option that's already there? but maybe not good enough as it's not an array of regexps. But actually my problem is the reverse. I need simple-oauth2 to be treated as external. And then all node_modules except the rest under server/node_modules.

I realized that the webpack externals functions do not work like middleware, so my code above would be missing some of what's in createConfigAsync.js.

[therealgilles]

Something like this would work (but it's awkward):

      const fnResMatch = razzleOptions.externalResMatch
        && razzleOptions.externalResMatch(res);

      ...
      if ((baseRes !== res) && !fnResMatch) {
        return callback();
      }
      ...
      if (res.match(/node_modules[/\\].*\.js$/) && fnResMatch) {

so that I can do this:

    // do not externalize the server node modules except for simple-oauth2
    razzleOptions.externalResMatch = res => !res.match(/\/server\//) || res.match(/simple-oauth2/)

PS: razzleOptions seems to be a weird place to put this as you don't have access to env.{ target, dev }.

[therealgilles]

I've been looking at the isLocal code and I'm not sure it's doing exactly what it's supposed to do. For instance, this require:

node_modules/react-dom/server.node.js
  module.exports = require('./cjs/react-dom-server.node.development.js');

references node_modules/cjs/react-dom-server.node.development.js. Is that really a "local" file?

There seems to be a fair number of these files:

> not externalize local .../my-app-canary/node_modules/razzle-dev-utils/prettyNodeErrors.js
> not externalize local .../my-app-canary/node_modules/webpack/hot/poll.js?300
> not externalize local ./log
> not externalize local ./log-apply-result
> not externalize local ./log
> not externalize local ./lib/express
> not externalize local ./server.node
> not externalize local ./cjs/react.development.js
> not externalize local ./cjs/react-router-dom.js
> not externalize local ./application
> not externalize local ./router/route
> not externalize local ./router
> not externalize local ./request
> not externalize local ./response
> not externalize local ./middleware/query
> not externalize local ./cjs/react-dom-server.node.development.js
> not externalize local ./router
> not externalize local ./middleware/init
> not externalize local ./middleware/query
> not externalize local ./view
> not externalize local ./utils
> not externalize local ./layer
> not externalize local ./utils
> not externalize local ../node_modules/css-loader/dist/runtime/cssWithMappingToString.js
> not externalize local ../node_modules/css-loader/dist/runtime/api.js
> not externalize local ../node_modules/css-loader/dist/runtime/cssWithMappingToString.js
> not externalize local ../node_modules/css-loader/dist/runtime/api.js
> not externalize local ./route
> not externalize local ./layer

It works fine in both cases. It seems weird that the decision to put something in the bundle depends on how the code is written. I guess I am not clear what we are trying to put inside external. If it's all the node_modules libraries, then it's not quite working, as a fair number are identified as 'local' and excluded.

[fivethreeo]

Decided to not expose this as razzle options, but added as webpackOptions

https://github.com/jaredpalmer/razzle/blob/256fc592ab11b68839f0817c863c960ab571a6b4/packages/razzle/config/createConfigAsync.js#L281

https://github.com/jaredpalmer/razzle/blob/canary/packages/razzle/config/createConfigAsync.js#L591

[fivethreeo]

https://github.com/jaredpalmer/razzle/blob/canary/packages/razzle/config/createConfigAsync.js#L330

[fivethreeo]

https://github.com/jaredpalmer/razzle/releases/tag/v4.0.0-canary.10

[therealgilles]

Thanks for the release! I appreciate the new debug flag.

Any thoughts about isLocal? Is the goal to create the smallest bundle possible for the server? When reading the webpack-node-externals documentation, it seems that is its goal. This is what I get with/out isLocal:

116K build-no-isLocal
860K build

[fivethreeo]

Is local is only to find stuff that just exists on the current machine that has to be bundled

[fivethreeo]

But it may be that stuff that has been resolved before is bundled

[therealgilles]

Would it be possible to pass context as argument to webpackOptions.notNodeExternalResMatch()?
'request' is not quite enough in my case as I need to call resolveRequest to get the full path:

    webpackOptions.notNodeExternalResMatch = (request, context) => {
      try {
        const res = resolveRequest(request, `${context}/`);
      } catch (err) {
        return false;
      }
      if (res.match(/\/server\/node_modules\//)) {
        return true;
      }
      return false;
    };

    webpackOptions.nodeExternals = [{ 'simple-oauth2': `commonjs ${path.join(paths.appPath, 'server/node_modules/simple-oauth2')}` }];

[therealgilles]

For isLocal, I am wondering if there should not be a check that the resulting file is not inside a node_modules directory.

[fivethreeo]

https://github.com/jaredpalmer/razzle/releases/tag/v4.0.0-canary.11

The way to do it is to fix the require/resolve where they happen or fix special cases in webpackOptions.nodeExternals

  // should not be absolute should be relative to appPath
    webpackOptions.nodeExternals = [{ 'simple-oauth2': `commonjs ${path.join(paths.appPath, 'server/node_modules/simple-oauth2')}` }];

[therealgilles]

Did you mean relative to appBuild instead?

This works:

    webpackOptions.nodeExternals = [{ 'simple-oauth2': `commonjs ../server/node_modules/simple-oauth2')}` }];

[fivethreeo]

Um, now I am questioning myself. Something seems fishy about the externals. They seem to be absolute. That should not happen.

[therealgilles]

Yes, I restarted with a brand new create-razzle-app@canary to check if it was my doing but I see that too.

[fivethreeo]

Yes, I see why.

https://github.com/vercel/next.js/blob/canary/packages/next/build/webpack-config.ts#L703

I missed something 😲😀

[fivethreeo]

If you want to you can fix it and send a pull. Use “fix(razzle): message” commit message if you do. Going to bed now ..

[therealgilles]

I'm looking at the next.js code related to baseRes and see the following comment:

      // Bundled Node.js code is relocated without its node_modules tree.
      // This means we need to make sure its request resolves to the same
      // package that'll be available at runtime. If it's not identical,
      // we need to bundle the code (even if it _should_ be external).

I wonder if you really want a similar behavior here. Is there a reason to expect that the node_modules directories won't be there during deployment? I think if that's the intention, then a similar comment should be added to the razzle code. Without it, the code does not make sense.

[fivethreeo]

It could be that the module is hoisted from some other package in development (so works in dev and breaks in prod, misconfiguration) or is a module that only exists in development on purpose. But we might want to warn when this happens so it does not happen on accident.

[fivethreeo]

https://github.com/jaredpalmer/razzle/releases/tag/v4.0.0-canary.12

[therealgilles]

Summarizing my setup in case it helps others.

I have jsconfig.json as follows:

{ 
  "compilerOptions": {
    "baseUrl": "node_modules",
    "paths": {
      "AppSrc": ["../app/src"],
      "ServerSrc": ["../server/src"],
      "Config": ["../config"]
    }
  }
}

and razzle.config.js as follows:

// razzle.config.js

const path = require('path')

module.exports = {
  options: {
    verbose: true,
    debug: {
      // nodeExternals: true,
    },
  },

  modifyWebpackOptions({
    env: {
      target,
      dev,
      serverless,
    },
    webpackObject,
    options: {
      pluginOptions,
      razzleOptions,
      webpackOptions,
    },
    paths,
  }) {
    // webpack does not understand class fields (yet), so we need to keep this library external
    webpackOptions.nodeExternals = [
      { 'simple-oauth2': `commonjs ../server/node_modules/simple-oauth2` },
    ]

    return webpackOptions
  },

  modifyWebpackConfig({
    env: {
      target,
      dev,
      serverless,
    },
    webpackConfig,
    webpackObject,
    options: {
      razzleOptions,
      webpackOptions,
    },
    paths,
  }) {
    // tell webpack where to search for node_modules
    webpackConfig.resolve.modules = [
      path.resolve(__dirname, 'node_modules'),
      path.resolve(__dirname, 'app/node_modules'),
      path.resolve(__dirname, 'server/node_modules'),
      'node_modules',
    ]

    return webpackConfig
  },

  modifyPaths({
    webpackObject,
    options: {
      razzleOptions,
    },
    paths,
  }) {
    // configure paths for our directory structure
    paths.appServerJs = path.join(paths.appPath, 'server/src/expressServer')
    paths.appServerIndexJs = path.join(paths.appPath, 'server/src/index')
    paths.appClientIndexJs = path.join(paths.appPath, 'app/src/index')

    return paths
  },
}

I ended not needing webpackOptions.notNodeExternalResMatch but it may come handy to other setups. Thank you for all the help @fivethreeo, very much appreciated. Now onto figuring out my proxy setup and what to do with 'window.' variables not available on node.

[fivethreeo]

Great! Glad I could help. Closing as resolved :)