Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Next.js 8 compatibility #8

Closed
martpie opened this issue Jan 25, 2019 · 22 comments
Closed

Next.js 8 compatibility #8

martpie opened this issue Jan 25, 2019 · 22 comments
Assignees

Comments

@martpie
Copy link
Owner

martpie commented Jan 25, 2019

I've heard reports that this plugin does not work with Next 8 (at the the time in canary version).

Need investigation.

Related: #1

@martpie
Copy link
Owner Author

martpie commented Jan 27, 2019

@azizhk @tsirlucas I just tested this plugin with v8 canary-17 and had no problems, can you confirm?

@martpie
Copy link
Owner Author

martpie commented Jan 27, 2019

I did not test with https://github.com/tsirlucas/next-transpile-errors though, will try in a moment

Edit: could reproduce it, investigating

@istarkov
Copy link

For our codebase solution provided here works well (next@8.0.0-canary.17)
All import errors have gone.

@martpie
Copy link
Owner Author

martpie commented Jan 28, 2019

Great, thank you for the feedback, I will add this change to the plugin, test a little bit more and draft a new release then.

Out of curiosity:

  • Why use Next Canary?
  • Can I know more about your setup? I could not reproduce this problem with my usual monorepo setup

@istarkov
Copy link

Why use Next Canary?

We are using google cloud functions for nextjs apps hosting, and one issue is too long cold start time - near 6-8 sec with current nextjs version. Seems like a lot of loadable dependencies have gone from next@canary in release version, (no webpack etc) So I'm checking now cold start times improvements have no results yet, just started

Can I know more about your setup?

We are using yarn with "workspaces" and your plugin allowed us to skip the build step for all internal libraries. Will try to create short example and publish soon.

@martpie
Copy link
Owner Author

martpie commented Jan 28, 2019

Thank you for the feedback!

@martpie
Copy link
Owner Author

martpie commented Jan 28, 2019

Can you confirm 1.1.0 solves the issue?

@istarkov
Copy link

At least for our app all is working with 1.1.0
Thank you!!!

@martpie
Copy link
Owner Author

martpie commented Jan 28, 2019

I will let this issue open until Next.js 8 is officially released.

@tsirlucas
Copy link

Hey @martpie, all import errors are gone now. Thanks for the quick fix and sorry for the delay to answer! The sample repo still have an error but I guess its related to babel/webpack an not to this plugin. Will try to fix everything and ping you if I notice that the problem its here.

@martpie
Copy link
Owner Author

martpie commented Jan 29, 2019

I guess we can close this then. Feel free to comment if you face additional problems.

@martpie martpie closed this as completed Jan 29, 2019
@tsirlucas
Copy link

Hey @martpie, I tried to fix the errors but had no success. I'll give you some more intel:

Why use Next Canary?

I'm trying to deploy to now 2.0 with a serverless setup and the canary version added the target: serverless setup which should solve a bunch of problems. Plus, even if I'm using the 7.0.2 version I still face the same error (but only when deploying to now 2.0).

Can I know more about your setup?

The repo I created adds an ui library that does some imports on .png files, so I need to transpile that library in order to make it work with nextjs.
The library keeps the folder structure when transpile, it is not bundled into a single file, so inside of that library we have an index file that imports the other files.

Before the files imported inside the library were with wrong paths, but after your fixes they're ok. We still get the error that we used to have when deploying to now 2.0, but if we are using the canary version, it does happen when you run yarn dev and try to access any page (on 7.0.2 development env used to work fine).

The error we are getting: TypeError: Cannot read property 'hexColor' of undefined

hexColor is a property of one of the modules that are being imported on our ui-library, I checked if something was wrong with the imports, but the library works well on all other applications and even on nextjs 7.0.2. This error only happens when deploying to now 2 with serverless, or when we are using nextjs canary, which introduces the target: serverless config.

Maybe the error is related to how next bundles the application for serverless deployment? Running yarn build + yarn start on 7.0.2 also works fine.

@azizhk
Copy link

azizhk commented Feb 1, 2019

Hi @martpie,
This might be because we are actually wrapping around next's external function and in case of serverless the external functions array is empty.

Relevant Next.js code:
https://github.com/zeit/next.js/blob/59280f7747b807fa9d1f5810e1d762d4f147124c/packages/next/build/webpack-config.js#L25-L31

Relevant next-plugin-transpile-modules code:
https://github.com/martpie/next-plugin-transpile-modules/blob/c85c175d9fdf122fddddbaa9d8fc14ec6d4407b0/index.js#L30-L34

We might want to add our external function if !config.externals || !config.externals.length

@martpie martpie reopened this Feb 1, 2019
@martpie
Copy link
Owner Author

martpie commented Feb 4, 2019

I am not sure to know what you mean @azizhk, would you mind submitting a PR? :) Then we can ask @tsirlucas to test the branch :)

@azizhk
Copy link

azizhk commented Feb 4, 2019

Ok, I'll see if I can figure it out. I won't get some time to give a PR this week but will try by next week.

@martpie
Copy link
Owner Author

martpie commented Feb 7, 2019

If this error is related to now 2.0, I am going to open a specific issue for it.

@martpie martpie closed this as completed Feb 7, 2019
@tsirlucas
Copy link

I think it is mostly related to serverless support than now 2. If you run the dev command with serverless target, the build will fail on the newer versions. Even locally.

@oliviertassinari
Copy link

oliviertassinari commented Feb 11, 2019

I had to apply this change to make Material-UI documentation works with Next.js v8.
@istarkov Thank you for linking it! The pull request.

@martpie
Copy link
Owner Author

martpie commented Feb 11, 2019

@oliviertassinari This change was released in next-plugin-transpile-modules@1.1.0, so you should not need to patch things yourself.

@heysailor
Copy link

heysailor commented Feb 27, 2019

I have it happily working in Next 8. It took some work to make react-native-web be happy though, as its use requires aliasing from react-native to react-native-web. Modules referenced from within react-native-web were unable to be found, despite the alias being made in the webpack output.

I couldn't quite drill down to why, but suspected the aliasing was incorrect for nested modules.

For those googling, I added to the default babel plugins in next.config.js:

const withPlugins = require('next-compose-plugins');
const withTypescript = require('@zeit/next-typescript');
const withTM = require('next-transpile-modules');

module.exports = withPlugins(
  [
    [
      withTM,
      {
        transpileModules: ['@expo/react-native-responsive-image'] // Replace as needed
      }
    ],
    withTypescript
  ],
  {
    webpack: (config, { defaultLoaders }) => {
      // Alias all `react-native` imports to `react-native-web`
      config.resolve.alias = {
        ...(config.resolve.alias || {}),
        'react-native$': 'react-native-web'
      };

      defaultLoaders.babel.options.plugins = [
        ['react-native-web', { commonjs: true }]
      ];

      return config;
    },
  }
);

.babelrc:

{
  "presets": ["next/babel", "@zeit/next-typescript/babel"],
  "plugins": [
    [
      "module-resolver",
      {
        "root": ["./"],
        "alias": {
          "^react-native$": "react-native-web"
        }
      }
    ]
  ]
}

@elmpp
Copy link

elmpp commented Jun 13, 2019

For anyone following the trails here and still having problems after @heysailor v.helpful post above - ensure the package being transpiled hasn't been hoisted by yarn if using a workspace. Add a nohoist rule as described here

(also ensure you are using a babel.config.js file rather than .babelrc file as described here

@Aleksion
Copy link

Aleksion commented Sep 6, 2019

@elmpp but what I need to hoist the package? In order to not have version clashes between packages in the repo?

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

No branches or pull requests

8 participants