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

Can't run locally for modifications #42

Closed
RReverser opened this issue Oct 25, 2015 · 11 comments
Closed

Can't run locally for modifications #42

RReverser opened this issue Oct 25, 2015 · 11 comments

Comments

@RReverser
Copy link
Collaborator

Tried to build & run locally, but got:

TypeError: $def is not a function in browser console in following context:

loadjs.d("323",function(require,module,exports){
// 19.1.3.1 Object.assign(target, source)
var $def = require('./$.def');

$def($def.S + $def.F, 'Object', {assign: require('./$.assign')});
},{"./$.assign":210,"./$.def":219});

loadjs.d("949",function(require,module,exports){
@fkling
Copy link
Owner

fkling commented Oct 26, 2015

Yeah, I know this issue. I'm not sure what causes it though. Using uglifyify used to solve this, I think because of dead code elimination, but maybe not anymore?

This is part of core-js. Maybe there is an issue with circular dependencies, not sure. @zloirock, have you seen this before?

@fkling
Copy link
Owner

fkling commented Oct 26, 2015

Uh, look like I removed uglifyify from npm run watch in one of the latest commits. Will add it back, but it would still be great to find a real solution, because uglifyify slows down the build...

fkling added a commit that referenced this issue Oct 26, 2015
to fix local builds (see #42)
@zloirock
Copy link

have you seen this before?

Sure, and each time it was mistake in build configuration. For example, babel not ignored babel-runtime.

@fkling
Copy link
Owner

fkling commented Oct 26, 2015

After looking a bit more into the generated files, it seems that $.def is required from dist/app.js, but is only defined in dist/parsers/babel.js.

Not sure why it is not defined in dist/app.js. I expected that any direct dependency of the entry file needs is also contained in it. @arian, am I doing something wrong here?

@arian
Copy link

arian commented Oct 26, 2015

How do you reprodue this? I just ran esprima_ast_explorer locally, using npm run watch, and the $.def module ends up in dist/app.js, just like the object.assign module.

Does updating to the latest partition-bundle help? 2.2.0 fixed some problems with deduped modules.

@fkling
Copy link
Owner

fkling commented Oct 26, 2015

Looks like I'm already using 2.2.1. I'm using

watchify -p [partition-bundle --map partition.json --output ./ --main ./src/app] \
-o /dev/null -v

It looks like there are two ./$.def modules, one with ID 650 (this one) and one with 218 (this one), i.e. they are definitely different modules. 650 is indeed contained in dist/app.js, but 218 is not (not sure if you'd get the same numbers in your build).

The are quite a few modules in dist/app.js referencing 218, e.g.

loadjs.d("322",function(require,module,exports){
// 19.1.3.1 Object.assign(target, source)
var $def = require('./$.def');

$def($def.S + $def.F, 'Object', {assign: require('./$.assign')});
},{"./$.assign":209,"./$.def":218});

but loadjs.d("218", ...) is in dist/parsers/babel.js.

I guess reusing the same (relative) module name shouldn't matter?

@fkling
Copy link
Owner

fkling commented Oct 30, 2015

I'll check if using webpack would solve this.

@RReverser
Copy link
Collaborator Author

@fkling Yes please! Currently scripts section using combination of browserify with some NIX-specific things so that I couldn't even contribute from own Windows machine despite this being a JS project :(

@fkling
Copy link
Owner

fkling commented Nov 21, 2015

I switched to webpack in 89db32f. It seems to work well so far. Let me know how it works for you.

@fkling fkling closed this as completed Nov 21, 2015
@RReverser
Copy link
Collaborator Author

Still seeing a bunch of webpack warnings on build (like, trying to require .md files somewhere and so on), but other than that, seems to build fine. Thanks!

@fkling
Copy link
Owner

fkling commented Nov 21, 2015

Yeah, I didn't figure out where it would try to load an .md file, but everything worked for me so I didn't bother with it too much.

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

4 participants