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

🐛 wrong babel usage #482

Closed
nnnikolay opened this issue Jan 3, 2018 · 15 comments
Closed

🐛 wrong babel usage #482

nnnikolay opened this issue Jan 3, 2018 · 15 comments

Comments

@nnnikolay
Copy link

I don't know is that a bug report or my mistake, but whole day today I'm struggling and get different behavior from time to time.

This is my .babelrc

{
  "presets": [
    "env",
    "stage-1",
    "react"
  ],
  "plugins": ["transform-react-jsx"],
  "ignore": [
    "node_modules"
  ]
}

and this is the message what I see

Server running at http://localhost:1234
🚨  /.../node_modules/attr-accept/dist/index.js: [BABEL] /.../node_modules/attr-accept/dist/index.js: Using removed Babel 5 option: /.../node_modules/attr-accept/.babelrc.stage - Check out the corresponding stag    at Logger.error (/.../node_modules/babel-core/lib/transformation/file/logger.js:41:11)
    at OptionManager.mergeOptions (/.../node_modules/babel-core/lib/transformation/file/options/option-manager.js:220:20)
    at OptionManager.init (/.../node_modules/babel-core/lib/transformation/file/options/option-manager.js:368:12)
    at File.initOptions (/.../node_modules/babel-core/lib/transformation/file/index.js:212:65)
    at new File (/.../node_modules/babel-core/lib/transformation/file/index.js:135:24)
    at JSAsset.getParserOptions (/.../node_modules/parcel-bundler/src/assets/JSAsset.js:60:20)
    at <anonymous>

that happens when I use react-dropzone package. Looks like that parcel/babel transpile node_modules as well?

here is my package.json

  "dependencies": {
    "prop-types": "15.6.0",
    "react": "16.1.0",
    "react-dom": "^16.2.0",
    "react-dropzone": "4.2.3",
    "react-pdf": "2.5.2"
  },
  "devDependencies": {
    "babel-core": "^6.26.0",
    "babel-plugin-add-module-exports": "0.2.1",
    "babel-plugin-transform-react-jsx": "^6.24.1",
    "babel-preset-env": "^1.6.1",
    "babel-preset-react": "6.24.1",
    "babel-preset-stage-1": "6.24.1",
    "cross-env": "^5.1.1",
    "enzyme-adapter-react-16": "^1.1.0",
    "eslint": "4.14.0",
    "eslint-config-airbnb": "16.1.0",
    "eslint-plugin-import": "2.8.0",
    "eslint-plugin-jsx-a11y": "6.0.3",
    "eslint-plugin-react": "7.5.1",
    "jest": "^21.2.1",
    "node-sass": "^4.7.2",
    "parcel-bundler": "^1.1.0",
    "prettier": "^1.9.1"
  }

as soon as I don't use that package everything is ok.

 parcel --version
1.4.1
@nnnikolay nnnikolay changed the title Babel exclude does not work? Babel "ignore" does not work? Jan 3, 2018
@nnnikolay nnnikolay changed the title Babel "ignore" does not work? Babel is not ignoring node_modules? Jan 3, 2018
@nnnikolay nnnikolay changed the title Babel is not ignoring node_modules? Babel is ignoring node_modules? Jan 3, 2018
@nnnikolay nnnikolay changed the title Babel is ignoring node_modules? Babel does not ignoring node_modules Jan 3, 2018
@nnnikolay
Copy link
Author

Well, looks like that parcel does not take into account content of the .babelrc which is located in the root project's folder at all. I mean it takes but only for the files which are located close to it.

Instead of that parcel takes each and every file for the bundle creation and during this process transpiles them with babel help. The main criteria to transpile is that the file .babelrc exists in the module folder or above but not above node_modules.

There are two questions from me

  1. Why do we need to transpile node_modules content at all???
  2. There is a case when package might have a transpiled version in ./node_modules/.../packageX/dist/index.js which is in use by the dependency of my dependency, but because of the logic described above and .babelrc file existence in the packageX root folder, parcel going to transpile it one more time.

@nnnikolay nnnikolay changed the title Babel does not ignoring node_modules 🐛 wrong babel usage Jan 4, 2018
@DeMoorJasper
Copy link
Member

DeMoorJasper commented Jan 4, 2018

It takes into account the first .babelrc it sees, so in this case the one from react-dropzone

@nnnikolay
Copy link
Author

nnnikolay commented Jan 4, 2018

@DeMoorJasper

async function resolve(filepath, filenames, root = path.parse(filepath).root) {
  filepath = path.dirname(filepath);

  // Don't traverse above the module root
  if (filepath === root || path.basename(filepath) === 'node_modules') {
    return null;
  }

  for (const filename of filenames) {
    let file = path.join(filepath, filename);
    let exists = existsCache.has(file)
      ? existsCache.get(file)
      : await fs.exists(file);
    if (exists) {
      existsCache.set(file, true);
      return file;
    }

    existsCache.set(file, false);
  }

  return resolve(filepath, filenames, root);
}

Right first, but if there is no .babelrc close to js then parcel goes further to the upper folder as you can see above

Still, the question is the same, why node_modules needs to be transpiled???

@DeMoorJasper
Copy link
Member

Yeah that's the point, if there is non in node_modules it goes up to project root but never past that so that makes sense.
I'm not sure what the use case with transpiling node_modules using the node_modules babel config is

@nnnikolay
Copy link
Author

@DeMoorJasper

if there is non in node_modules it goes up to project

Where do you see any validation that the asset is inside of node_modules?

I'm not sure what the use case with transpiling node_modules

The same for me, I don't know why do you guys transpile node_modules content at all

@DeMoorJasper
Copy link
Member

@nnnikolay there is no validation if it's in node_modules or not, this piece of code prevents traversing outside of the project though:

// Don't traverse above the module root
  if (filepath === root || path.basename(filepath) === 'node_modules') {
    return null;
  }

@nnnikolay
Copy link
Author

@DeMoorJasper not only the project but also module/package folder as well.

Sorry if I confused you with my explanation, let me try one more time.

Parcel takes file by file to build the bundle. In the process, there are couple of stages

async process() {
    if (!this.generated) {
      await this.loadIfNeeded();
      await this.pretransform();
      await this.getDependencies();
      await this.transform();
      this.generated = this.generate();
      this.hash = this.generateHash();
    }

    return this.generated;
  }

Some of the assets are located inside node_modules folder, for example, this one /ProjectRootFolder/node_modules/attr-accept/dist/index.js. When it's time to process such asset, parcel will look for .babelrc in the same folder and will keep doing that until either found one or achieve node_modules folder. The problem is that the package attr-accept has .babelrc file in the package's root folder, nevertheless there is no any reason to transpile dist/index.js.

@azz
Copy link

azz commented Jan 5, 2018

Having a similar issue - is there a way to disable babel on node_modules?

Couldn't find preset "es2015" relative to directory ...

🚨  /Users/azz/code/npm-ui/node_modules/react-input-autosize/lib/AutosizeInput.js: Couldn't find preset "es2015" relative to directory "/Users/azz/code/npm-ui/node_modules/react-input-autosize"
    at /Users/azz/code/npm-ui/node_modules/babel-core/lib/transformation/file/options/option-manager.js:293:19
[1]     at Array.map (<anonymous>)
[1]     at OptionManager.resolvePresets (/Users/azz/code/npm-ui/node_modules/babel-core/lib/transformation/file/options/option-manager.js:275:20)
[1]     at OptionManager.mergePresets (/Users/azz/code/npm-ui/node_modules/babel-core/lib/transformation/file/options/option-manager.js:264:10)
[1]     at OptionManager.mergeOptions (/Users/azz/code/npm-ui/node_modules/babel-core/lib/transformation/file/options/option-manager.js:249:14)
[1]     at OptionManager.init (/Users/azz/code/npm-ui/node_modules/babel-core/lib/transformation/file/options/option-manager.js:368:12)
[1]     at File.initOptions (/Users/azz/code/npm-ui/node_modules/babel-core/lib/transformation/file/index.js:212:65)
[1]     at new File (/Users/azz/code/npm-ui/node_modules/babel-core/lib/transformation/file/index.js:135:24)
[1]     at JSAsset.getParserOptions (/Users/azz/code/npm-ui/node_modules/parcel-bundler/src/assets/JSAsset.js:60:20)
[1]     at <anonymous>

@nnnikolay
Copy link
Author

nnnikolay commented Jan 5, 2018

@azz This is exactly the issue what I'm talking about here, and yep package react-input-autosize has .babelrc in module's root folder.
it's still not ok to transpile node_modules in my opinion.

Here is one more example

./node_modules/react-dropzone/dist/es/index.js: Unknown plugin "add-module-exports" specified in "./node_modules/react-dropzone/.babelrc.env.development" at 0, attempted to resolve relative to "./node_modules/react-dropzone"

It's enough to check that the path of the current asset (asset.name) contains a node_modules substring. And if it contains then we do not need to transpile the asset.

I've tried to do something like that

async function shouldTransform(asset) {
  if (asset.name.indexOf('node_modules') !== -1) {
    return false;
  }

  if (asset.isES6Module) {
    return true;
  }

  if (asset.ast) {
    return !!asset.babelConfig;
  }

  if (asset.package && asset.package.babel) {
    return true;
  }

  let babelrc = await config.resolve(asset.name, ['.babelrc', '.babelrc.js']);
  return !!babelrc;
}

but it does not work somehow :(

@digitaltopo
Copy link

Same in #515

@stychu
Copy link

stychu commented Jan 11, 2018

I've also encountered this problem. Im trying to create some starter app with react and material-ui. With the latest version parcel is working great. But when I want to use material-ui@next the problem with babelrc shows up. There are multiple packages in nodemodules that contains Babelrc file which parcel tries to ctranspile and this is causing crash preventing my app to run. After deleting all Babelrc files from modules app successfully starts up bit then there are some problem with importing components from material-ui :(

@ajsharp
Copy link

ajsharp commented Feb 8, 2018

Having the same issue. It seems many dependencies -- whether they should or not -- package .babelrc with their npm package. I agree with others on this thread that parcel should probably not build or pay any attention to dependencies' build configuration.

Moreover, speaking as someone trying to use parcel on a new project, this issue is common enough that it makes for a pretty poor first-time experience if you hit it. Regardless of whether parcel or the underlying libraries are at fault, parcel is the only build system that exposes this problem. It would be a welcome change to have this fixed.

@DeMoorJasper Is there some core parcel functionality that relies on this behavior? Also, I have a .babelrc in my project root and parcel still picks up dependency .babelrc files.

FWIW, I ran into this problem here: react-dropzone/attr-accept#16.

@ajsharp
Copy link

ajsharp commented Feb 8, 2018

For anyone else who stumbles on this issue, the main discussion thread for the underlying issue is #13. This issue should likely be closed.

@devongovett
Copy link
Member

In v1.6.0, we no longer transpile node_modules by default. Closing.

@Offirmo
Copy link

Offirmo commented Jun 15, 2018

@devongovett This is a controversial move. I know that create-react-app does the same and expects node_modules to be pre-transpiled, but that disputable. How would node_modules know which level of transpilation is needed? Should we all transpile to ES4 / IE8 forever?

Is there a way to reactivate transpilation/babelification of node_modules?

[edit] just found your contribution #1101 which suits my needs. Will try now.
[edit] I was indeed in a monorepo situation, and the "source" field made the trick.

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