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

Doesn't play nice with pre-rollupify transforms (e.g. jsxify, coffeeify) #1

Open
nolanlawson opened this issue Mar 4, 2016 · 9 comments

Comments

@nolanlawson
Copy link
Owner

E.g. JSX files that refer each other via import. Not sure if it's best to call that a "wontfix" or if there's some way to crawl the dependency graph and apply other Browserify transforms before this one.

@ansballard
Copy link

Would this go along with trying to use something like partialify to import html partials? Could there be an option to ignore specific file extensions, then run through babelify to transpile the rest of the import and export statements? Hopefully that makes sense, will try to clarify if not.

@nolanlawson
Copy link
Owner Author

I'm already whitelisting certain file extensions, but yeah, this could be extended to other file types. I'd be curious to know what babelify currently does for ES6 modules.

@ansballard
Copy link

Ah my bad, missed that. My question is more about imports nested within others, which is more of a rollup issue. I can get around it passing a glob of html paths within the entry directory to rollup.external, then calling babelify and partialify afterwards.

glob(path.resolve(path.dirname(filename) + "/**/*.html"), function(err, paths) { // added
  writeFile(tmpfile, source, 'utf8').then(function () {
    return rollup.rollup({
      entry: tmpfile,
      sourceMap: doSourceMap ? 'inline' : false,
      external: paths // added
    })
...

Seems a little heavy handed, and only handles one filetype, but maybe there's a better way to handle it? Didn't see a better way to ignore file extensions in the rollup docs.

@nolanlawson
Copy link
Owner Author

Sorry, I don't quite understand your question. I mean this module has a whitelist of extensions, so it ought to ignore .html by default. That sounds like it isn't what you want, though.

BTW circling back around to this issue, I can confirm that JSX files imported by other JSX files do not work; i.e. this module does not play nice with jsxify. For now I'm going to call this a "wontfix" because it's a pretty non-trivial change to detect other transforms ahead of rollupify and tell Rollup to apply those transforms on submodules.

Unexpected token (3:10) in /Users/nlawson/workspace-node/rollupify/test/test4/index.jsx.tmp while parsing file: /Users/nlawson/workspace-node/rollupify/test/test4/index.jsx
  SyntaxError: Unexpected token (3:10) in test/test4/index.jsx.tmp while parsing file: /Users/nlawson/workspace-node/rollupify/test/test4/index.jsx

Another workaround is to just use Rollup separately. E.g. build with Babel but without babel-plugin-transform-es2015-modules-commonjs, then use that as the input to Rollup.

@nolanlawson nolanlawson changed the title Probably doesn't work if you have pre-rollupify transforms in submodules Doesn't play nice with pre-rollupify transforms (e.g. jsxify, coffeeify) Mar 4, 2016
nolanlawson added a commit that referenced this issue Mar 4, 2016
@nolanlawson
Copy link
Owner Author

^ there's a failing test if anyone is interested in taking this up

@jessehattabaugh
Copy link

I tried Browserifying without babel-plugin-transform-es2015-modules-commonjs but then I got Error: Failed to create bundle: 'import' and 'export' may appear only with 'sourceType: module'. Then I went and got coffee and thought about it and realized that it's impossible to use Browserify without having something transpile the ES6 modules to CJS.

So I guess what you meant is to forego Browserify entirely, and let Rollup handle it? My question is if I go this route, will Rollup do the same job as envify?

@ansballard
Copy link

@nolanlawson Actually your jsx issue is the exact same as what I'm trying to describe. I was thinking non-js modules could be skipped and passed on to babelify to be transformed, but rollup does't output the directory structure so that doesn't work without detecting transforms for each non-js module, which as you said is nontrivial. tl;dr same as the jsx issue, rollupify won't apply transforms before itself.

@nolanlawson
Copy link
Owner Author

@jessehattabaugh yeah assuming your code is valid ES6, you can run it through Rollup on the command line and then run that output through browserify. Probably less error-prone than rollupify in your case.

@ansballard yeah it's a tricky issue. not sure how to proceed TBH

@ansballard
Copy link

#3 would actually resolve this one as well, at least for transforms that rollup has plugins for. So browserify -t babelify in.js > out.js would become browserify -t [rollupify --plugins [ rollup-babel] ] in.js > out.js. So the user would have to know which transforms need to be switched to use rollup, but it should work.

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

3 participants