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

Transform es2015 modules if our babel preset runs outside of Next.js #1095

Closed

Conversation

arunoda
Copy link
Contributor

@arunoda arunoda commented Feb 12, 2017

Fixes #1042

Usually next/babel is used outside of Next.js. That's why basically we exposed it.
But it cause a lot of issues lately because, we don't transform ES2015 modules.

This causes issues when used with Jest and with cases like #1042

So, with this PR we changed our preset to transform ES2015 everywhere except inside Next.js.

"presets": ["es2015", "next/babel"]
}
}
"presets": [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't get this, we've to use a crazy .babelrc like this to support Jest.

// (And that's how it can do code splitting)
//
// But in other environements like Jest, we should transform it.
if (process.env.INSIDE_NEXT) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be not the best solution. But this is the best option I could find out.

I tried using passing an option to our babel config. But, when the user adds a custom .babelrc, he/she has to add that option manually. Which is not nice.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if you add a config option to disable module transpilation 🤔 So they are enabled by default when no options are given. And in our code (webpack.js) we pass the disabled option. Would that work? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed to @timneutkens

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then the problem when used with Jest. Both Jest and Next.js reads from the same config.
So, we have to add different env and provide different babel setups.

If we go this approach, there's no need to take this PR at all.
We can simply ask them to use es2015 + next/babel

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding more to this:

Everything works until someone expose a custom babelrc.
So, we've to document a lot to make this work.

Copy link
Contributor Author

@arunoda arunoda Feb 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaredpalmer in this case, we were looking for targetting specific env like test. But users could use this even in production to do something like this.

Copy link
Contributor

@nkzawa nkzawa Feb 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arunoda do you mean this #1044 ? Can you elaborate what the problem is?

Copy link
Contributor Author

@arunoda arunoda Feb 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkzawa I mean this: #1042
See: https://github.com/angel200sdnot/sample_repo/blob/master/package.json#L17

With that, he/she is transpiling server.js using next/babel. It works before not now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we have to modify user defined babel config to disable module compilation ? but I think it'd be cleaner way if we can do that. Anyway it would be nice to have options for our babel preset.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkzawa how about this:

  • We allow user to provide some options to our babel config (to transform modules or not)
  • If don't provide ^^^ option and runs inside Next.js we won't transform ES2015 modules?

@jaredpalmer
Copy link
Contributor

This has prevented my team from moving off of beta 20. Thank you for this.

@@ -12,6 +12,10 @@ import getConfig from '../config'
import * as babelCore from 'babel-core'
import findBabelConfigLocation from './babel/find-config-location'

// Mark whether we are inside Next.js
// So, our babel preset could do some Next.js specific configurations
process.env.INSIDE_NEXT = 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔

@arunoda arunoda added this to the 2.1 milestone Mar 12, 2017
@timneutkens
Copy link
Member

@arunoda I haven't heard this come up again, should we close it?

@timneutkens
Copy link
Member

Lets close this.

@lock lock bot locked as resolved and limited conversation to collaborators May 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants