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

Development Bundles doesn't work #1227

Closed
1 task done
pYr0x opened this issue Jul 25, 2017 · 15 comments
Closed
1 task done

Development Bundles doesn't work #1227

pYr0x opened this issue Jul 25, 2017 · 15 comments

Comments

@pYr0x
Copy link
Collaborator

pYr0x commented Jul 25, 2017

How often can you reproduce it?

  • Always

Description:
i installed steal and steal-tools on a fresh project.
i want to create a deps bundle. i followed the guides https://stealjs.com/docs/StealJS.guides.development_bundles.html#add-npm-script-to-generate-the-bundle and added the new script tag <script src="node_modules/steal/steal.js" deps-bundle></script>

Steps to reproduce:

  1. clone repo https://github.com/pYr0x/jss-steal-app
  2. npm install
  3. npm run deps-bundle

Expected results:
the dev.html is working.

Actual results:
i get an error:

TypeError: define is not a function
	    at Object.eval (http://localhost:63342/jss-steal-app/dev-bundle.js:4:110682)
	    at eval (http://localhost:63342/jss-steal-app/dev-bundle.js:5:4)
	    at eval (http://localhost:63342/jss-steal-app/dev-bundle.js:5:98)
	Evaluating http://localhost:63342/jss-steal-app/dev-bundle.js null

Environment:

Software Version
Steal version 1.5.5
Steal-tools version 1.7.0
node -v 6
npm -v 3.10
Browser chrome
Operating system mac
@pYr0x
Copy link
Collaborator Author

pYr0x commented Jul 26, 2017

Any suggestions @m-mujica ?

@m-mujica
Copy link
Contributor

@pYr0x I haven't looked into this yet.... Once I'm done with my current issue I'll see what I can do, I'll let you know.

@pYr0x
Copy link
Collaborator Author

pYr0x commented Jul 26, 2017

Ok thanks

@m-mujica
Copy link
Contributor

@pYr0x I can see the error, seems to be caused by the minifier.

I'm looking deep into it but while I get a fix you could just pass --no-minify to the bundle command and it should load fine (and it should still be faster that loading each dep individually)

"deps-bundle": "steal-tools bundle --deps --no-minify"

Thanks for the example app, I'll update this issue once I find/fix the problem.

@matthewp
Copy link
Member

Am I wrong in thinking that dev bundles are unminified by default?

@m-mujica
Copy link
Contributor

@matthewp I can't remember, looking at the code now. 😬

@m-mujica
Copy link
Contributor

@m-mujica
Copy link
Contributor

@matthewp the piece of code the bundle command adds to preload the bundled packages breaks the comments regex applied to the source code in the amd extension (the package json files sometime include glob patters like "/*.js" and that breaks the regex)

sourceWithoutComments = load.source.replace(strictCommentRegEx, '$1'),

So the bundle's format is set to cjs instead of amd.

@pYr0x
Copy link
Collaborator Author

pYr0x commented Jul 27, 2017

I wonder if there is no test for the deps bundle?
Can we not set that midule pre definded as amd, with the meta config ?

@m-mujica
Copy link
Contributor

m-mujica commented Jul 27, 2017

I wonder if there is no test for the deps bundle?

sure, there are tests https://github.com/stealjs/steal-tools/blob/master/test/dev_bundle_build_test.js, the bug will show up only if a dependency package.json has a `"/*" in one of its properties.

Can we not set that midule pre definded as amd, with the meta config ?

maybe, not sure how though.

@pYr0x
Copy link
Collaborator Author

pYr0x commented Jul 27, 2017

@matthewp
Copy link
Member

matthewp commented Dec 6, 2017

Should dev bundles be unminified? I can't think of when/why you'd want them to be minified. Just takes longer to build that way.

@m-mujica
Copy link
Contributor

m-mujica commented Dec 7, 2017

I think we should have made that the default, I can't remember why we did not do it.

I'm in to change it though.

@matthewp
Copy link
Member

matthewp commented Dec 9, 2017

So if we just make that the default that takes away this bug, right?

@m-mujica
Copy link
Contributor

m-mujica commented Dec 9, 2017

@matthewp yup, there you go stealjs/steal-tools#905

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants