Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Babel not working for library code #1046

Closed
piu130 opened this issue Aug 26, 2018 · 11 comments
Closed

Babel not working for library code #1046

piu130 opened this issue Aug 26, 2018 · 11 comments
Labels

Comments

@piu130
Copy link
Contributor

piu130 commented Aug 26, 2018

Bug?

  • What version of Neutrino are you using?
    "neutrino": "^8.3.0"

  • Are you trying to use any presets? If so, which ones, and what versions?
    "@neutrinojs/react": "^8.3.0"

  • Are you using the Yarn client or the npm client? What version?
    "yarn": "1.9.4"

  • What version of Node.js are you using?
    "node": "10.8.0"

  • What did you do?
    I created a project with npx @neutrinojs/create-project test, and added a library (react-native-elements) that uses object spread syntax.

  • What did you expect to happen?
    It should compile and display the website.

  • What actually happened, contrary to your expectations?
    This error (and many others):

Module parse failed: Unexpected token (9:4)
You may need an appropriate loader to handle this file type.

const styles = StyleSheet.create({
  text: {
    ...Platform.select({ // <---- error here (spread syntax)
      android: {
        ...fonts.android.regular,

How do I configure babel to support library code?

@edmorley
Copy link
Member

Hi!

The @neutrinojs/react v8 preset does support spread syntax, since it is based on @neutrinojs/web:

babel: compileLoader.merge({
plugins: [
...(options.polyfills.async ? [[require.resolve('fast-async'), { spec: true }]] : []),
require.resolve('babel-plugin-syntax-dynamic-import'),
require.resolve('babel-plugin-transform-object-rest-spread')
],

There's probably something else going wrong - could you post a complete testcase repo?

@piu130
Copy link
Contributor Author

piu130 commented Aug 26, 2018

I think the problem is that babel excludes node_modules folder by default. Object spread works in src folder.
I tried to add those lines to .neutrinorc.js to include react-native-elements with no success:

compile: {
  exclude: [/node_modules(?!\/react-native-elements)/]
}

Project setup:
npx @neutrinojs/create-project test
yarn add react-native-web react-native-elements@1.0.0-beta5 react-native-vector-icons@^4.2.0

After adding import { Button } from 'react-native-elements' to App.jsx it throws the error.

@edmorley
Copy link
Member

edmorley commented Aug 26, 2018

There are a few parts to this:

  1. Neutrino does not compile node_modules by default, since packages should be compiling down to ES5 themselves (even when using the module property in package.json). There is an increasing (and unfortunate) trend within the react-native packages to not do this. See Enable babel-preset-env for node_modules that target newer Node versions facebook/create-react-app#1125 and Compile dependencies with babel-preset-env facebook/create-react-app#3776 for more details. For now, I'm not convinced Neutrino should transpile node_modules by default - especially since it's easily overridden by doing:
// .neutrinorc.js
const { join } = require('path');

module.exports = {
  use: [
    ['@neutrinojs/react', {
      // Any preset options here.
    }],
    // Use the Neutrino API to make changes that are not possible via the preset's options.
    (neutrino) => {
      // Also Babel transpile node_modules (in addition to the project source/tests).
      neutrino.config.module
        .rule('compile')
          .include.add(join(__dirname, 'node_modules'));
    },
  ],
};
  1. Modern Node.js (>=8.3.0) and many modern browsers (https://kangax.github.io/compat-table/es2016plus/#test-object_rest/spread_properties) already support spread syntax. The reason this case is failing, is since webpack v3 uses an older version of acorn which doesn't. The soon to be released Neutrino 9 uses webpack 4, which doesn't have this limitation - and therefore wouldn't need node_modules to be transpiled iff you are only targetting browsers that support spread syntax.

@piu130
Copy link
Contributor Author

piu130 commented Aug 26, 2018

Thanks for your explanation.

  1. The code above gives me the error:
Module build failed: ReferenceError: [BABEL] /home/le/develop/web/test/src/index.jsx: Unknown option: base.include. Check out http://babeljs.io/docs/usage/options/ for more information about options.

So I tried with, which results in the initial error:

compile: {
  include: [ join(__dirname, 'node_modules', 'react-native-elements') ]
}
  1. It's not only the spread syntax making problems. There is also class properties and jsx and maybe others.

@edmorley
Copy link
Member

edmorley commented Aug 26, 2018

Ah sorry I misread the preset here:

neutrino.use(compileLoader, {
include: [
neutrino.options.source,
neutrino.options.tests
],
exclude: [staticDir],
babel: options.babel
});

Adding the node_modules directory actually requires using the more advanced Neutrino API (since it's not available via the preset options) - I've updated the example above. Let me know if that doesn't work :-)

@piu130 piu130 closed this as completed Aug 27, 2018
@edmorley
Copy link
Member

@eliperelman, I can think of a few things we can do to help with this:

  1. Add documentation explaining how to use the Neutrino API to add node_modules (either to the @neutrinojs/compile-loader docs, or to individual top-level presets).
  2. Add an option to @neutrinojs/web and others, that either lets people specify additional directories, or perhaps is just an includeNodeModules bool? Though I'm not sure where this option would go, since we pass options.babel directly to babel-loader, and it doesn't seem great to hijack that and filter the option out the other end?

@eliperelman
Copy link
Member

My preference is definitely (1), since we have spent a lot of time reducing top-level options that just duplicate other functionality.

@edmorley
Copy link
Member

I've filed #1100 for adding this to the docs.

@piu130
Copy link
Contributor Author

piu130 commented Sep 28, 2018

@edmorley @eliperelman should we add a node_modules/react-native-* regex in the react preset so it would be easier to use react-native-web for neutrino newcomers?

@eliperelman
Copy link
Member

@piu130 feel free to open a PR and we can discuss it there.

@edmorley
Copy link
Member

Trying to auto-detect whether a project is using packages that don't compile down to ES5 by looking around node_modules seems like it might be both fragile and surprising to the user. eg: if someone had react-native-something in their deps and then removed it, their build might suddenly start failing if they also had a package named differently that also needed compiling.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

3 participants