martpie / next-transpile-modules

Next.js plugin to transpile code from node_modules. Please see: https://github.com/martpie/next-transpile-modules/issues/291
MIT License
1.13k stars 85 forks source link

Next.js 7 / Webpack 4 compatibility #1

Closed martpie closed 5 years ago

martpie commented 6 years ago

This plugin oddly does not work with Next 7. I spent multiple hours trying to figure out what was going wrong without success.

Refs:

I still do not know if the problem comes from Webpack or Next's custom Babel loaders.

Any hints/help would be highly appreciated.


Temporary solutions:

lucleray commented 6 years ago

I tried it with next 7 and it worked (specifically I tried this : https://github.com/zeit/next.js/tree/canary/examples/with-yarn-workspaces).

What is the error you are seeing ?

martpie commented 6 years ago

Oh is it? I got errors when using it with Next's withTypescript.

I should write tests for both scenarios, it should not be too hard.

martpie commented 6 years ago

screen shot 2018-09-26 at 14 29 17

And I also saw this comment saying it was not working: https://github.com/zeit/next.js/pull/3732#issuecomment-423302026

lucleray commented 6 years ago

@martpie The bug is definitely related to Typescript then.

Have you tried replacing this :

module.exports = withTypescript(
  withTM({
    transpileModules: ['somemodule', 'oranother']
  })
)

by this :

module.exports = withTM(
  withTypescript({
    transpileModules: ['somemodule', 'oranother']
  })
)

?

alexanbj commented 6 years ago

I don't think this has something to do with TS. I don't use TS and it doesn't work for me either with Next 7.

martpie commented 6 years ago

I don’t think the problem comes from Webpack but Babel... I will have to dive into Next’s build codebase.

If anyone has interesting inputs/advices... feel free! 😅

edit: I am more than sure the problem comes from Next's Babel now. Some Webpack 4 guys told me everything looked fine, when using functions instead of regex for includes/excludes, the loader is correctly chosen. So something in the loader is wrong.

azizhk commented 6 years ago

I was getting this error:

/node_modules/@babel/runtime/helpers/esm/extends.js:1
(function (exports, require, module, __filename, __dirname) { export default function _extends() {
                                                              ^^^^^^

SyntaxError: Unexpected token export
    at createScript (vm.js:80:10)
    at Object.runInThisContext (vm.js:139:10)
    at Module._compile (module.js:616:28)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)
    at Module.require (module.js:596:17)
    at require (internal/module.js:11:18)

I comented the externals part and got my build working. https://github.com/martpie/next-plugin-transpile-modules/blob/fcabad4cc8b15524f75c5ab266e3644847436175/index.js#L30-L34

So, as you are saying that webpack guys are saying that the plugin is correct, I'm looking at the value of externals coming from next, now. Will share if I find anything.

azizhk commented 6 years ago
node_modules/
  want_to_transpile/
    node_modules/
      dont_want_to_transpile/
  third_party/
  next/

So I think this might be the problem. My dont_want_to_transpile module which is a dependency of want_to_transpile module is being bundled by webpack. (Which it should not).

Now that dont_want_to_transpileis requiring third_party module which is not bundled (expected) But it is expecting that third_party module be transpiled.

But at bundling time, it resolved to a non-transpiled (esm) source code. So it assumed that path and saved it while bundling dont_want_to_transpile module.

So I think webpack needs to give us fine grained control over which bundling want_to_transpile and marking want_to_transpile/node_modules/* as external.

azizhk commented 6 years ago

And as to why my build works when I comment externals code is because my want_to_transpile module also has es2015 code published in dist.

But for a module which does not publish es2015 code, commenting externals would not work. So my solution wont-fix.

martpie commented 6 years ago

I faced some problems with node_modules regexs too. If I want to transpile shared, node_modules/any-other-module/shared will be transpiled.

I can replace the include/exclude with custom functions instead of regexps. It should make the code more readable too. But I still get the transpilation problem.

morajabi commented 6 years ago

I have the same issue, but for striping "flow" types in a monorepo:

- monorepo root
   - shared // <- trying to transpile this
   - web
   | - next.config.js

It doesn't transpile imports from shared.

morajabi commented 6 years ago

Looks like it doesn't pick up the correct ".babelrc" because of a breaking change on how babel looks for config.

martpie commented 6 years ago

@morajabi So are you saying this is a problem in the user's setup or Next's setup?

morajabi commented 6 years ago

Not user setup most likely. I think we need to figure out a way to adopt to the new Babel behaviour in Next 7.

azizhk commented 6 years ago

Create react app version 2 is transpiling npm packages. We can learn from there.

reicheltp commented 6 years ago

It does not pick the .babelrc. As a temporary workaround you can add your babel presets / plugins like this:

const withTranspileModules = require('next-plugin-transpile-modules')
module.exports = withTranspileModules({
  transpileModules: ['@my_namespace'],
  webpack: (config, { defaultLoaders }) => {
    // this shoudl also work with @babel/typescript
    defaultLoaders.babel.options.presets = ['@babel/flow']
    defaultLoaders.babel.options.plugins = [
      ['react-native-web', { commonjs: true }],
    ]

    return config
  },
})
martpie commented 6 years ago

Similar to https://github.com/zeit/next.js/issues/5288

morajabi commented 6 years ago

Thanks @reicheltp @martpie, the temporary workaround worked perfectly for me. (monorepo)

martpie commented 6 years ago

Ok, after countless hours reading changelogs and poorly documented Babel 7 changes, I managed to transpile a local symlinked package from node_modules. The secret is to declare a babel.config.js with some rules in it.

I do not like it because it is just rewriting config files that should be done by Next already, but waiting for a cleaner solution, I'll update the package and the documentation soon.

martpie commented 6 years ago

For those interested, the quick fix (actually quite similar to @reicheltp solution):

// babel.config.js (in your Next.js folder)
module.exports = (api) => {
  api.cache(true);

  // adapt this to your setup
  const presets = [
    'next/babel',
    '@zeit/next-typescript/babel' // if you use TypeScript
  ];

  return {
    presets
  };
}

I am checking if I can do this automatically via the plugin.

And here's a monorepo example: https://github.com/martpie/monorepo-typescript-next-the-sane-way

edit: I opened an issue on Next.js's repo, I will wait on their feedback before moving on.

mtnt commented 5 years ago

Any updates? I tried make a fix as shown at https://github.com/martpie/next-plugin-transpile-modules/issues/1#issuecomment-427749256, but still got an error:

I use propTypes from some child component to declare parent`s one and got Cannot read property '__propName__' of undefined

martpie commented 5 years ago

@Mtnt Do you have a piece of code to share so I can reproduce this error? (a component or something)

mtnt commented 5 years ago

@martpie

It`s an assignment of props for npm module "@sample-ns/controlls"

const { type: controlType, ...controlProps } = Control.propTypes;
const { type: groupType, ...groupProps } = Group.propTypes;

Checkbox.propTypes = controlProps;
Radio.propTypes = controlProps;
RadioGroup.propTypes = groupProps;
CheckboxGroup.propTypes = groupProps;

next.config.js

module.exports = withPlugins([
  [
    withTM,
    {transpileModules: ['@sample-ns']},
  ],
]);

So, it`s transformed to

var _Control$propTypes = control.propTypes,
    controlType = _Control$propTypes.type,
    controlProps = _objectWithoutProperties(_Control$propTypes, ["type"]);

var _Group$propTypes = group.propTypes,
    groupType = _Group$propTypes.type,
    groupProps = _objectWithoutProperties(_Group$propTypes, ["type"]);

and fires an exception on _Control$propTypes.type

is it enough?

martpie commented 5 years ago

Have you checked if it was working well with Next.js 6?

mtnt commented 5 years ago

@martpie wow, suddenly - it did not oO. I missed a commit that added some changes. Sorry...

However, there is still a problem... that prop types did not cropped. I created the 'type' variable only for omitting it in Checkbox.propType and expected all code removed. Any advice for fast fix?

TxHawks commented 5 years ago

I'm not sure if this is the same issue, but i get the following error while server rendering:

SyntaxError: Unexpected identifier
    at new Script (vm.js:74:7)
    at createScript (vm.js:246:10)
    at Object.runInThisContext (vm.js:298:10)
    at Module._compile (internal/modules/cjs/loader.js:657:28)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:20:18)
(node:2892) UnhandledPromiseRejectionWarning: /project-root/node_modules/@babel/runtime/helpers/esm/objectSpread.js:1
(function (exports, require, module, __filename, __dirname) { import defineProperty from "./defineProperty";

and then, of course, an empty response on the client.

curran commented 5 years ago

FWIW here's an example that shows transpiling working with Next 7, but not watching.

https://github.com/curran/nextjs-esm-example/pull/4

Struggling to get watching to work.

Tracked as #5

tsirlucas commented 5 years ago

Temporary solutions stoped working on most recent canary versions (even in 7.0.2 canary versions, but also in 8.0.0 most recent one)... For some reason if the transpiled dependency entry file imports some code from subfolders, we end up with a unstranspiled import on our app.

Instead of importing from node_modules/dependency/components/ComponentA we end up with ./components/ComponentA and that leads to import errors.

Also, tried this library with the new target: serverless option and it seems not to work for some reason. Maybe this plugin shouldn't work on serverless environment? Maybe its not working due to the import error that im getting locally.

I'll try to create a repo reproducing both cases and post it here

martpie commented 5 years ago

I did not play with Next 8 yet but I subscribed to the releases. I am going to investigate over the week-end where things fail.

tsirlucas commented 5 years ago

Here is the repo with the 7.0.2-canary.31 version where we can already reproduce the import problems, so I guess the problem its between 7.0.2 and 7.0.2-canary.31

https://github.com/tsirlucas/next-transpile-errors.

To reproduce the same thing in the 8.0.0 version you can just update it to 8.0.0-canary.13. Also, the commit adding serverless target its on 7.0.2-canary.50, so I guess the problem is not really the serverless config.

azizhk commented 5 years ago

So I went through @tsirlucas project. I changed https://github.com/martpie/next-plugin-transpile-modules/blob/c85c175d9fdf122fddddbaa9d8fc14ec6d4407b0/index.js#L30-L34 to

const path = require('path')
config.externals = config.externals.map(external => {
  if (typeof external !== 'function') return external
  return (ctx, req, cb) => {
    return includes.find(include => 
      req.startsWith('.')
        ? include.test(path.resolve(ctx, req))
        : include.test(req)
    )
      ? cb()
      : external(ctx, req, cb);
  }
})

And got some things working. Though am just shooting in the dark here.

martpie commented 5 years ago

Let's move the discussion about Next 8 in #8 ;)

martpie commented 5 years ago

After talking to Next.js maintainers, it is actually not a bug, when you use plugins that use babel, like typescript or flow, you need to update your .babelrc accordingly as documented in the plugins. I will update the readme and close this issue.