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

Babelify config for Browserify does not work with rc10 #329

Closed
kahlil opened this issue Nov 14, 2017 · 18 comments
Closed

Babelify config for Browserify does not work with rc10 #329

kahlil opened this issue Nov 14, 2017 · 18 comments

Comments

@kahlil
Copy link

kahlil commented Nov 14, 2017

I use the following Browserify config:

"browserify": {
	"transform": [
		[
			"babelify",
			{
				"presets": [
					"env"
				]
			}
		]
	]
}

And Bankai can't parse the import syntax:

Failed while processing 'scripts'.

 --> index.js:2:1
  |
2 | import { html, render } from 'lit-html';
  | ^ Syntax Error

Hmmm. We're having trouble parsing a file.

The necessary modules are installed.

@yoshuawuyts
Copy link
Member

Oh, that's odd. I would've expected shama/yo-yoify#58 by @goto-bus-stop to have solved our final compat issues. My guess is there's probably some parser in the tree that's operating on an outdated version.

Also, on a side note we're going to be baking in Babel support for stable language features sometime in the next 2 weeks. Hopefully that'll fix this issue in the long run C:

@kahlil
Copy link
Author

kahlil commented Nov 14, 2017

👍 for baking Babel in! Still this simple Browserify config should still work, right?

@yoshuawuyts
Copy link
Member

Yeah, I would've expected it to! Not sure why it isn't parsing the import syntax. Here's a project where we're using browserify transforms too. So I don't think it's being glossed over, ghmmm. I'm still suspecting a transform using an outdated parser somewhere.

Could you perhaps run bankai build to see if it crashes with a different error? Thanks!

@kahlil
Copy link
Author

kahlil commented Nov 14, 2017

This is the output on bankai build:

❯ npx bankai build
14:30:28 ✨  Compiling & compressing files

14:30:28 ✨  created: dist
14:30:28 🚨  bankai.asset: could not find a file for 
14:30:28 ✨  created: dist/manifest.json
14:30:28 ✨  created: dist/manifest.json.gz
14:30:28 ✨  created: dist/manifest.json.deflate
14:30:28 ✨  created: dist/manifest.json.br
14:30:28 🚨  Failed while processing 'scripts'.

 --> index.js:2:1
  |
2 | import { html, render } from 'lit-html';
  | ^ Syntax Error

Hmmm. We're having trouble parsing a file.

@yoshuawuyts
Copy link
Member

Ugh; looks like we did too good of a job on prettifying errors >.>

Let me try and repro this. Be back with you in a bit!

@kahlil
Copy link
Author

kahlil commented Nov 14, 2017

Thanks!

@yoshuawuyts
Copy link
Member

yoshuawuyts commented Nov 14, 2017

Okay, looks like brfs is the culprit here. For a quick fix, apply the following patch to bankai:

diff --git a/lib/graph-script.js b/lib/graph-script.js
index 86d345a..2e4d083 100644
--- a/lib/graph-script.js
+++ b/lib/graph-script.js
@@ -37,7 +37,7 @@ function node (state, createEdge) {
 
   b.ignore('sheetify/insert')
   b.transform(sheetify)
-  b.transform(brfs)
+  // b.transform(brfs)
   b.transform(glslify)
   b.transform(yoyoify, { global: true })
 
@@ -51,6 +51,7 @@ function node (state, createEdge) {
     self.emit('progress', 'scripts', 30)
 
     b.bundle(function (err, bundle) {
+      console.log('error', err)
       if (err) {
         delete err.stream
         err = ttyError('scripts', 'browserify.bundle', err)

Recommended to run without switching buffers through the -q flag.

$ bankai start -q

It seems after this patch is applied there's still some errors related to importing source type module style modules from within Node. So there may be a few more hurdles, ugh. Let me dig a little further on how to solve this, ghmmm.

edit: note that this may just be an issue with lit-html, because they don't have the .mjs syntax, and Node doesn't like that. We should find a way past that.

@kahlil
Copy link
Author

kahlil commented Nov 14, 2017

Thanks Yosh.

@yoshuawuyts
Copy link
Member

Oh, no yeah think I figured it out — I think the transform needs to be global because lit-html uses the export keyword, and locally defined transforms don't operate globally. Alright, let's get Babel support into core! @kahlil any presets you generally use?

@goto-bus-stop
Copy link
Member

goto-bus-stop commented Nov 14, 2017 via email

@kahlil
Copy link
Author

kahlil commented Nov 14, 2017

oh yeah yeah that's not an issue I just added the same config into the lit-html package.json to make that work. A global transform is not advisable I think.

My problem was just that the transform did not apply to my file.

When it comes to presets I am just interested in the default env presets.
I think what would be interesting as well would be rollup support :)

@yoshuawuyts
Copy link
Member

Waiting for brfs & static-module to be moved to the Browserify org. Related patches:


I think what would be interesting as well would be rollup support :)

@kahlil Oh cool! — what would you like to see from Rollup? We have tree shaking, bundle flattening, and soon code splitting too (in #266). Would we be missing anything? (:

@kahlil
Copy link
Author

kahlil commented Nov 14, 2017

We have tree shaking, bundle flattening, and soon code splitting too (in #266). Would we be missing anything? (:

Oh, well don't mind me then, that sounds perfect 😄 .

@yoshuawuyts yoshuawuyts mentioned this issue Nov 14, 2017
@yoshuawuyts
Copy link
Member

Added a PR to add Babelify into Bankai directly here #332 🎉 — If this doesn't land straight away, we should at least be publishing the brfs patch within the next 24 hours. Thanks!

@kahlil
Copy link
Author

kahlil commented Nov 14, 2017 via email

@yoshuawuyts
Copy link
Member

[email protected] has been published; no babel yet, but it should work with the transforms in package.json at least.

We're probably going to release rc12 with Babel support & code splitting in the next few days, with bankai 9 stable (hopefully) before the end of the week 🎉

@kahlil
Copy link
Author

kahlil commented Nov 15, 2017

Works-a like-a deh charm-a! (Mario voice)

🙏 thanks y'allz are super.

@yoshuawuyts
Copy link
Member

Yay, glad to hear!

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