FormidableLabs / babel-plugin-transform-define

Compile time code replacement for babel similar to Webpack's DefinePlugin
MIT License
245 stars 31 forks source link

Issue with babel 7 #43

Closed Vincz closed 5 years ago

Vincz commented 6 years ago

When we define a config file like that :

{
    "plugins": [["transform-define", "./config/config.js"]]
}

We get the following error with babel 7 :

.plugins[1][1] must be an object, false, or undefined

Vincz commented 6 years ago

What about something like that : https://github.com/Vincz/babel-plugin-transform-define/commit/5ba90a6ad8ba8fc4fa9b791d80409db0b1c3ab3d

Or maybe __file instead of file

We cannot use a string as plugin configuration anymore. So we have to use an object even for the file import feature and we need to know if the user want to have an object as config or if he wants to use a config file.

I used to use it like that in my .babelrc :

"env": {
    "development": {
        "plugins": ["react-hot-loader/babel", ["transform-define", "./config/config.dev.js"]]
    },
    "production": {
        "plugins": [["transform-define", "./config/config.prod.js"]]
    }
}
binary64 commented 6 years ago

thanks Vincz, I'm trying to test your fork. I edited my package.json like so: "babel-plugin-transform-define": "git@github.com:Vincz/babel-plugin-transform-define.git" and npm install runs fine, but npm run dev kicks up a run-time error:

> next

{ Error: Cannot find module 'babel-plugin-transform-define' from 'C:\p\myapp\conf\pods\adminsite'
    at Function.module.exports [as sync] (C:\p\myapp\conf\pods\adminsite\node_modules\resolve\lib\sync.js:40:15)
    at resolveStandardizedName (C:\p\myapp\conf\pods\adminsite\node_modules\@babel\core\lib\config\files\plugins.js:78:29)
    at resolvePlugin (C:\p\myapp\conf\pods\adminsite\node_modules\@babel\core\lib\config\files\plugins.js:27:10)
    at loadPlugin (C:\p\myapp\conf\pods\adminsite\node_modules\@babel\core\lib\config\files\plugins.js:35:18)
    at createDescriptor (C:\p\myapp\conf\pods\adminsite\node_modules\@babel\core\lib\config\config-descriptors.js:152:21)
    at C:\p\myapp\conf\pods\adminsite\node_modules\@babel\core\lib\config\config-descriptors.js:104:12
    at Array.map (<anonymous>)
    at createDescriptors (C:\p\myapp\conf\pods\adminsite\node_modules\@babel\core\lib\config\config-descriptors.js:103:27)
    at createPluginDescriptors (C:\p\myapp\conf\pods\adminsite\node_modules\@babel\core\lib\config\config-descriptors.js:99:10)
    at C:\p\myapp\conf\pods\adminsite\node_modules\@babel\core\lib\config\config-descriptors.js:50:19
    at plugins (C:\p\myapp\conf\pods\adminsite\node_modules\@babel\core\lib\config\config-descriptors.js:40:25)
    at mergeChainOpts (C:\p\myapp\conf\pods\adminsite\node_modules\@babel\core\lib\config\config-chain.js:296:68)
    at C:\p\myapp\conf\pods\adminsite\node_modules\@babel\core\lib\config\config-chain.js:251:7
    at buildRootChain (C:\p\myapp\conf\pods\adminsite\node_modules\@babel\core\lib\config\config-chain.js:85:20)
    at loadPrivatePartialConfig (C:\p\myapp\conf\pods\adminsite\node_modules\@babel\core\lib\config\partial.js:41:53)
    at loadPartialConfig (C:\p\myapp\conf\pods\adminsite\node_modules\@babel\core\lib\config\partial.js:66:16) code: 'MODULE_NOT_FOUND' }
dcalhoun commented 6 years ago

@Vincz I've run into this same issue. Is there a reason you have not opened a PR with your proposed changes? Thanks!

Vincz commented 6 years ago

@binary64 You need to compile the package first (run npm run build in the module folder). @dcalhoun The first reason is that it's a breaking change, so it'll break if updated without updating the plugin configuration. The second is that, as we now need to use an object, we cannot really know if the user wants the config object to be the "define" object or if the object contains a path to a file contains the "define" object.

dcalhoun commented 6 years ago

@Vincz that makes sense, but opening a PR would at least prompt a discussion from this plugin's maintainers. It appears this issue has gone largely unnoticed by the maintainers. Hopefully a PR to address this breakage would gain more attention.

In regards to your first point, I understand it is a breaking change, but it's already broken in Babel 7, so this change would likely be necessary. If I understand it correctly, support for passing a string as a babel plugin option was never intended, but was more recently rejection has been enforced with type checking.

To your second point, I believe we could likely create an API for this plugin that would allow for continued support for both an inline config object and a file path. It would be easier to discuss that in a PR where the proposed changes are showcased though.

Thanks for taking time to explore a fix!

xcarpentier commented 6 years ago

I just create a PR with @Vincz changes plus fixed tests => https://github.com/FormidableLabs/babel-plugin-transform-define/pull/49

And I published it here on npm: https://www.npmjs.com/package/babel-plugin-transform-define-file

I try in my side, but I'm still have issue on nextjs 6.0.0...

Let me know.

EDIT.

With nextjs 6, work fine if update "babel-preset-env": "7.0.0-beta.3"

Vincz commented 6 years ago

Thank you @xcarpentier !

chepelevdi commented 6 years ago

@Vincz @xcarpentier Hi. Please can you tell me what am I doing wrong?

"devDependencies": {
    "babel-plugin-transform-define": "^1.3.0",
    "babel-plugin-transform-define-file": "^1.3.2",
  }
  "dependencies": {
    "next": "6.0.1-canary.2",
    "react": "^16.3.2",
    "react-dom": "^16.3.2",
  },

.babel.rc

{
  "presets": [
    "next/babel"
  ],
  "plugins": [
    [
      "transform-define",
      {
        filePath: "./env-config.js"
      }
    ]
  ]
}

env-config

const prod = process.env.NODE_ENV === 'production'
module.exports = {
  'process.env.BACKEND_URL': prod ? '/' : 'http://localhost:5000'
}

But in my component console.log('proc', process.env.BACKEND_URL) - undefined. It was working in next 5((

xcarpentier commented 6 years ago

Hi @chepelevdi You should wait the PR merge but for now you can remove babel-plugin-transform-define and change filePath to file.

chepelevdi commented 6 years ago

@xcarpentier tnx a lot

nathanqueija commented 6 years ago

Hey, guys.

I managed how to fix that passing an object config to this plugin using .babelrc.js instead of .babelrc:

const env = require('./env-config')
module.exports = {
  presets: [['@babel/preset-env', {modules: 'commonjs'}], 'next/babel'],
  plugins: [
    ['lodash'],
    ['transform-define', env],
    ['transform-decorators-legacy'],
    [
      'inline-import',
      {
        extensions: ['.css']
      }
    ],
    [
      'babel-plugin-styled-components',
      {
        ssr: true
      }
    ]
  ],
  env: {
    test: {},
    development: {},
    production: {}
  }
}
agungsb commented 6 years ago

@nathanqueija 's solution works for me. Thanks, @nathanqueija .. 🎉

julkue commented 6 years ago

@nathanqueija Thanks, works for me too.

pmg1989 commented 6 years ago

works for me too. Thanks, @nathanqueija