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

fix dev code for webpack compatibility #346

Merged
merged 6 commits into from
Jul 3, 2018
Merged

Conversation

cherifGsoul
Copy link
Member

This fixes the first part of this issue canjs/canjs#4170 to remove debug code from the production builds.

@cherifGsoul
Copy link
Member Author

@matthewp @justinbmeyer is this can be merged?

can-define.js Outdated
// If value is a constructor, give a warning
if (definition.default && canReflect.isConstructorLike(definition.default)) {
canLogDev.warn("can-define: The \"default\" for " + prop + " is set to a constructor. Did you mean \"Default\" instead?");
if(process.env.NODE_ENV !== 'production') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's an indentation problem here.

list/list.js Outdated
@@ -590,44 +599,46 @@ var DefineList = Construct.extend("DefineList",
// pass `key` to only log the matching event, e.g: `list.log("add")`
log: function(key) {
//!steal-remove-start
var instance = this;
if(process.env.NODE_ENV !== 'production') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might it have been easier to have log be a no-op here and at the bottom of this file:

//!steal-remove...
if(process.env...){
  DefineList.prototype.log = function(){ ... }
}
//!steal-remove..

map/map.js Outdated
var quoteString = function quoteString(x) {
return typeof x === "string" ? JSON.stringify(x) : x;
};
if(process.env.NODE_ENV !== 'production') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment about log from above. Btw, could log be shared?

Copy link
Member Author

@cherifGsoul cherifGsoul Jun 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They could be shared even if they don't share the same event types?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log is shared now.

@cherifGsoul
Copy link
Member Author

@justinbmeyer log function is in helpers now

@justinbmeyer
Copy link
Contributor

justinbmeyer commented Jul 3, 2018

the ensure_meta should probably be put in define_helpers too. I think we should avoid creating modules unnecessarily. It adds to the loadtime.

@cherifGsoul
Copy link
Member Author

@justinbmeyer Can we handle ensure_meta in other PR?

@justinbmeyer
Copy link
Contributor

justinbmeyer commented Jul 3, 2018 via email

@cherifGsoul cherifGsoul merged commit 0d6ce7d into master Jul 3, 2018
@cherifGsoul cherifGsoul deleted the fix-webpack-debug-style branch July 3, 2018 23:42
This pull request was closed.
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

Successfully merging this pull request may close these issues.

3 participants