Skip to content

Conversation

@gjreasoner
Copy link
Contributor

@gjreasoner gjreasoner commented Oct 20, 2017

Fixes #267

Requiring just markdown like so

require('markdown')

Includes a file like so

exports.markdown = require("./markdown");
exports.parse = exports.markdown.toHTML;

Which to the best of my knowledge exposes markdown as require('markdown').markdown which requires the need to assign it to a variable and then to use it outside of the bundled code you'll want to assign it to window.

https://github.com/gjrdiesel/portal/blob/834d506705b88add765cef26280a5d86e01ab6a3/resources/assets/js/bootstrap.js#L62

@gjreasoner
Copy link
Contributor Author

I thought I'd tack one more on here while I was changing the webpack.mix.js file; hope thats ok.


if (mix.config.inProduction) {
mix.version();
}
Copy link
Member

Choose a reason for hiding this comment

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

Why wouldn't you want to do this locally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The way the npm build command works you could still do versioning locally if you wanted; In Laravel Mix, mix.config.inProduction is an indicator if your running npm run production or npm run dev.

What this enables is npm run hot which provided by Laravel Mix, watches for file/css changes and then automatically rebuilds app.js or app.css file when they do change. The issue is if you use mix.version and npm run hot, you get chunk errors, this conditional fixes that.

So you can still run mix.version() locally by running npm run production.

Copy link
Member

Choose a reason for hiding this comment

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

Ok thanks for the explanation. Any reason why this isn't in the mix file by default?

Copy link
Member

Choose a reason for hiding this comment

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

Nvm. Just read the documentation for it.

@driesvints
Copy link
Member

Looks good 👌
Just one question.

Thanks for sending this in!

Copy link
Member

@driesvints driesvints left a comment

Choose a reason for hiding this comment

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

Nice job!


if (mix.config.inProduction) {
mix.version();
}
Copy link
Member

Choose a reason for hiding this comment

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

Nvm. Just read the documentation for it.

@driesvints driesvints merged commit 4275dd0 into laravelio:master Oct 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants