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

Issue with babel 7 #43

Closed
Vincz opened this issue Jan 10, 2018 · 14 comments · Fixed by #57
Closed

Issue with babel 7 #43

Vincz opened this issue Jan 10, 2018 · 14 comments · Fixed by #57

Comments

@Vincz
Copy link

Vincz commented Jan 10, 2018

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
Copy link
Author

Vincz commented Jan 10, 2018

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
Copy link

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
Copy link

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

@Vincz
Copy link
Author

Vincz commented Apr 30, 2018

@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
Copy link

@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
Copy link

xcarpentier commented Apr 30, 2018

I just create a PR with @Vincz changes plus fixed tests => #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
Copy link
Author

Vincz commented May 1, 2018

Thank you @xcarpentier !

@chepelevdi
Copy link

chepelevdi commented May 10, 2018

@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
Copy link

xcarpentier commented May 11, 2018

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

@chepelevdi
Copy link

@xcarpentier tnx a lot

@nathanqueija
Copy link

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
Copy link

agungsb commented May 18, 2018

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

@julkue
Copy link

julkue commented May 18, 2018

@nathanqueija Thanks, works for me too.

@pmg1989
Copy link

pmg1989 commented May 31, 2018

works for me too. Thanks, @nathanqueija

ryan-roemer added a commit that referenced this issue Oct 23, 2019
* **BREAKING**: Change plugin options to **only** be a real JS object. Removes string configuration path option as now this is all possible with dynamic `.babelrc.js` or `babel.config.js` files. 
* **BREAKING**: Update to `@babel/core` / Babel 7+. Fixes #43 Closes #49, #50 
* Lint all `test` code. Most diffs in this PR are those changes. Fixes #56 
* Update other dependencies. Closes #55
* Remove dependency on babel6-only test library https://github.com/walmartlabs/assert-transform and do a simple function replacement.
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

Successfully merging a pull request may close this issue.

9 participants