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 block warnings when block is inside "code" #1590

Merged
merged 1 commit into from
Jul 16, 2014

Conversation

ForbesLindesay
Copy link
Member

[fixes #1589]

A layout like:

if condition
  block myBlock

should not emit a warning, since myBlock may be used.

ForbesLindesay added a commit that referenced this pull request Jul 16, 2014
Fix block warnings when block is inside "code"
@ForbesLindesay ForbesLindesay merged commit b46e4e9 into master Jul 16, 2014
@ForbesLindesay ForbesLindesay deleted the fix-1589-block-warnings branch July 16, 2014 22:40
@jbnicolai
Copy link
Member

Nice! Was just waiting for Travis to come back to his merge myself

Appreciate all the hard work you've been doing, @ForbesLindesay! Trying to contribute more actively myself as well, but struggling to find time..

Keep it up 👍

@ForbesLindesay
Copy link
Member Author

Thanks 👍. It's always good to have someone read over what I've done. I'll stop self-merging once we stop getting the regressions from 1.4.0 being reported. Hopefully we're through most of those by now.

@jbnicolai
Copy link
Member

Don't worry, self merge whenever you're confident and leave if open when you'd like a second pair of eyes.

@sveisvei
Copy link

Why cant blocks be used as future hooks? - erroring out on this seams harsh. Getting abit sick of jade being more and more anoying(warnings out in production logs etc) for each upgrade of version.
Maybe a strict mode should be the way to go?

@ForbesLindesay
Copy link
Member Author

Jade won't ever have multiple modes. Be that "strict" mode or otherwise. There is no feature for which that will be implemented. We already have "doctypes" as a kind of mode, and that will be it.

Sloppy mode is easy to get, just lock your version to a specific version of jade. JavaScript added modes because it was forced to maintain backwards compatibility. You will still get forced into strict mode once you start using newer features like modules and generators. Jade is the same, if you want the new features, you get the new warnings.

At this stage, these are just warnings, partially so that we can get feedback on this specific case and ensure that we aren't making a big mistake. If you have a genuine use case for "blocks that aren't ever used", tell us about it and we may allow it to not be an error in the future.

At the moment, I don't quite buy the use case of them being for future extensibility? Are you suggesting you define a block in a child template so that a layout can choose to use it or not at a later date? That seems kind of backwards to me.

@sveisvei
Copy link

@ForbesLindesay I think my comment may have been abit to harsh here. I really enjoy your making jade more sane.

(For me to get a bunch of nonimportant warnings when in production feels wrong, that is my point, but 100% understand that "modes" is not the way to go when I think about it.)

Concerning this case with blocks here, for my part it is reporting wrong in my application - because I have user selected templates who extends the layouts - so its hard to inspect.

@ForbesLindesay
Copy link
Member Author

I agree that it's a shame to have these warnings in production, but the options are either warnings or errors. Without the warnings, everyone will be hit very hard once they become errors in 2.0.0.

For your particular use case, perhaps you could take advantage of the fact that jade compilation/rendering is synchronous by doing something like:

var originalWarn = console.warn;
var warnings = [];
console.warn = function (str) {
  warnings.push(str);
};
var view = jade.compile(template);
console.warn = originalWarn;
// show warnings to user in appropriate ways
// render template

That way you could help you users to update their templates gradually.

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.

an error occurs in the first definition of the block when using 'if else'
3 participants