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

JSX support #6

Open
philjoseph opened this issue Jun 2, 2019 · 24 comments
Open

JSX support #6

philjoseph opened this issue Jun 2, 2019 · 24 comments
Labels
enhancement New feature or request

Comments

@philjoseph
Copy link

Hello - I am trying to run your utility on my react-native code and I hit and error with the JSX syntax

 [!] (commonjs plugin) SyntaxError: Unexpected token (35:12) in /Users/philc/dev/tkabel_mc/react/index.native.js
react/index.native.js (35:12)
33:     render() {
34:         return (
35:             <App { ...this.props } />
                ^
36:         );
37:     }
SyntaxError: Unexpected token (35:12) in /Users/philc/dev/tkabel_mc/react/index.native.js
    at Object.pp$4.raise (/Users/philc/.npm/_npx/61536/lib/node_modules/rollup/node_modules/acorn/dist/acorn.js:2844:13)
    at Object.pp.unexpected (/Users/philc/.npm/_npx/61536/lib/node_modules/rollup/node_modules/acorn/dist/acorn.js:690:8)
    at Object.pp$3.parseExprAtom (/Users/philc/.npm/_npx/61536/lib/node_modules/rollup/node_modules/acorn/dist/acorn.js:2292:10)
    at Object.parseExprAtom (/Users/philc/.npm/_npx/61536/lib/node_modules/rollup/dist/rollup.js:15022:26)
    at Object.parseExprAtom (/Users/philc/.npm/_npx/61536/lib/node_modules/rollup/dist/rollup.js:15039:30)
    at Object.pp$3.parseExprSubscripts (/Users/philc/.npm/_npx/61536/lib/node_modules/rollup/node_modules/acorn/dist/acorn.js:2126:19)
    at Object.pp$3.parseMaybeUnary (/Users/philc/.npm/_npx/61536/lib/node_modules/rollup/node_modules/acorn/dist/acorn.js:2103:17)
    at Object.pp$3.parseExprOps (/Users/philc/.npm/_npx/61536/lib/node_modules/rollup/node_modules/acorn/dist/acorn.js:2045:19)
    at Object.pp$3.parseMaybeConditional (/Users/philc/.npm/_npx/61536/lib/node_modules/rollup/node_modules/acorn/dist/acorn.js:2028:19)
    at Object.pp$3.parseMaybeAssign (/Users/philc/.npm/_npx/61536/lib/node_modules/rollup/node_modules/acorn/dist/acorn.js:2003:19)

I've tried to edit my .babel.rc to help but no impact. What shall I do ?

@spencermountain
Copy link
Owner

thanks, yeah I'd like to support jsx. Maybe it's easy.
I'll take a look at it this week.

@spencermountain spencermountain changed the title Running on react-native code JSX support Jun 3, 2019
@spencermountain spencermountain added the enhancement New feature or request label Jun 3, 2019
@spencermountain
Copy link
Owner

i've added the babel rollup plugin, but the jsx results seem to be weird still.
can you try 0.2.0?
thanks

@alspdx
Copy link

alspdx commented Jun 27, 2019

I just tried this in 0.2.0, I get the same error, SyntaxError: Unexpected Token ....

@mdobroggg
Copy link

Same issue with JSX.

@TimothyJones
Copy link

JSX support would be great. It seems like 0.2.0 isn't using the local .babelrc to do any transformations.

@TimothyJones
Copy link

Further to this - my jsx files had the extension .js. I tried renaming to .jsx, and got a different error:

[!] (plugin babel) Error: Runtime helpers are not enabled. Either exclude the transform-runtime Babel plugin or pass the `runtimeHelpers: true` option. See https://github.com/rollup/rollup-plugin-babel#configuring-babel for more information

@TimothyJones
Copy link

TimothyJones commented Sep 11, 2019

Further to this - I got it working, but had to check out and change rollup.config.js significantly. Here is a summary of the changes:

  • Put babel() before commonjs()
  • Put babel config directly in the babel() call, and exclude node_modules
  • Bring in several new rollup plugins (for CSS, etc), and rollup-plugin-alias to allow top level imports
  • Change resolve({ browser: true }) to work around an error with a module that reassigned require
  • Configure a lot of namedExports aliases for commonjs(), because it is super strict about the way modules are allowed to export

A fix could be to allow custom rollup config, but it might be better to let it work directly on a provided sourcemap (which I could generate with minimal changes to my webpack settings)

@spencermountain
Copy link
Owner

pr! pr!
thanks timothy. I've been dragging my heels on this.
Will merge anything!

@TimothyJones
Copy link

I'd love to make a PR, but since many of the changes were specific to the particular settings of our project, I don't think I can generalise the solution enough.

@OnkelTem
Copy link

ok, so no basically - no React support? :(
and it looked so promising...

@TimothyJones
Copy link

Generalised React support isn't straightforward. See my comment above for some pointers if you want to get it working just in your project, though.

@OnkelTem
Copy link

Generalised React support isn't straightforward. See my comment above for some pointers if you want to get it working just in your project, though.

I'm not sure its worth trying in my case, because we use webpack style loader plugin and this finder basically crashes on the first import styles from './style';

@elyobo
Copy link

elyobo commented Feb 18, 2020

@OnkelTem I had the same issue, so I tried trucker which seemed to do the job for me.

@OnkelTem
Copy link

@OnkelTem I had the same issue, so I tried trucker which seemed to do the job for me.

Thanks! Already on it. Unfortunately it doesn't work well :) Gonna create the issue about it.

@elyobo
Copy link

elyobo commented Feb 18, 2020

OK, @ me on the issue if you don't mind, I'm curious - worked well for me, in that I identified a few things that had been orphaned in refactoring and were no longer in use. Code base included plenty of JSX and newer ES syntax. Possibly there were false negatives though, unused code that it failed to detect.

@OnkelTem
Copy link

@elyobo Sure, you're welcome: davidmfoley/node-trucker#25

@spencermountain
Copy link
Owner

oooh! Trucker looks great!
Thank you for sharing that @elyobo .
It's using babel, right? I'd be happy to integrate/borrow/be-inspired-by their solution for crawling the source tree. Ours is not very clever.

@elyobo
Copy link

elyobo commented Feb 18, 2020

Thanks @OnkelTem, I see what your issue is there and why it's not a problem for us (we don't use that feature of webpack). IDK that you'll get much luck in having them support a webpack specific feature, but I could imagine them adding some sort of config to configure a set of paths to try resolving from which would allow a bundler agnostic way of supporting that.

@spencermountain it's not my tool and I haven't looked at the underlying implementation, so no comment about what it uses under the hood :) I think the identification of unused modules is really a side effect of the main purpose of the tool - moving files and updating the requires/imports for them, which would require them to understand the whole dependency structure and therefore identifying the unused ones would be pretty straightforward.

@OnkelTem
Copy link

OnkelTem commented Feb 19, 2020

@elyobo A crazy thought has come to mind: can't we make the file counting -- a part of webpack build process instead, implemented as a plugin maybe? Unfort. my knowledge of webpack is quite limited, but I guess there might be some callbacks to use. Not a bundler agnostic way, for sure :)

@elyobo
Copy link

elyobo commented Feb 19, 2020

Not crazy at all... so not crazy that it seems people are well ahead of us on this. Have not tried, but...

@OnkelTem
Copy link

@elyobo Come on, really! I haven't even bothered to search for this. Thanks again, you're a pioneer! :)

@OnkelTem
Copy link

OnkelTem commented Feb 19, 2020

So I've tested the first one:

and it works perfectly!

This is my configuration:

new UnusedWebpackPlugin({
    // Source directories
    directories: [path.join(ROOT_DIR, 'src')],
    // Exclude patterns
    exclude: [
        '*.jpg',
        '*.json',
        '/assets/',
        '/pages/common/icons',
        '/pages/common/react-md-components/*/__tests__/',
        '/pages/common/react-md-components/*/__mocks__/'
    ],
    // Root directory (optional)
    root: ROOT_DIR,
}),

It found 38 unused files out from ~1160.
Not that bad!

Finally I know that amount, because before it was totally obscure to me (I'm a new dev on this quite old project, and that it's old is clear from my exclusion of 'react-md-components' which was put under src/ because noone wished to bother with keeping the project up to date with that package LOL).

Btw, using it as a webpack plugin is even better: now every developer would see these warnings.

And I'm a real pain in the ass. Because this and my desire to perform global spell check makes my teammates nervous LOL.

@elyobo
Copy link

elyobo commented Feb 19, 2020 via email

@OnkelTem
Copy link

OnkelTem commented Feb 19, 2020

Russian team here. Our winners are: shedule, scedule, schedlue, premissions, permisions, togle, wraper, formated, succes and etc hehehe. 893 typos altogether! But some are professional terms not known to the spellcheker, some - wrongcamelCase or NOTPROPERLYSPLIT_CONSTANTS etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants