Skip to content
This repository was archived by the owner on Aug 20, 2018. It is now read-only.

Fix #43. Add string options to output json object string. #45

Merged
merged 3 commits into from
Mar 15, 2017
Merged

Conversation

HairyRabbit
Copy link
Contributor

output json object as string.

@jsf-clabot
Copy link

jsf-clabot commented Mar 14, 2017

CLA assistant check
All committers have signed the CLA.

index.js Outdated
return "module.exports = " + JSON.stringify(value) + ";";
this.cacheable && this.cacheable();
var value = typeof source === "string" ? JSON.parse(source) : source;
this.value = [value];
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated, but this can be removed since it was needed in webpack =< v0.8.0 only

Copy link

Choose a reason for hiding this comment

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

Yeah. this.cacheable isn't needed either.

README.md Outdated

### Options

#### `string`
Copy link
Member

Choose a reason for hiding this comment

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

string => stringify for naming consistency with JSON.stringify

index.js Outdated
this.value = [value];
var query = loaderUtils.getOptions(this) || {}
if(query.string)
return "module.exports = `" + JSON.stringify(value) + "`;";
Copy link
Member

Choose a reason for hiding this comment

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

Please put it on the same line as the if statement or add the {} 😛

Copy link

Choose a reason for hiding this comment

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

Sidenote - webpack-defaults will catch these.

README.md Outdated

#### `string`

By default, the json-loader will output the json object, set this query parameter to 'ture' can output the json object as a string, e.g. `require('json-loader?string!../index.json')`. It's useful work with `ExtractTextPlugin`
Copy link

Choose a reason for hiding this comment

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

true over ture.

Copy link

Choose a reason for hiding this comment

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

Tune the last sentence too.

index.js Outdated
return "module.exports = " + JSON.stringify(value) + ";";
this.cacheable && this.cacheable();
var value = typeof source === "string" ? JSON.parse(source) : source;
this.value = [value];
Copy link

Choose a reason for hiding this comment

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

Yeah. this.cacheable isn't needed either.

index.js Outdated
this.value = [value];
var query = loaderUtils.getOptions(this) || {}
if(query.string)
return "module.exports = `" + JSON.stringify(value) + "`;";
Copy link

Choose a reason for hiding this comment

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

Sidenote - webpack-defaults will catch these.

index.js Outdated
var query = loaderUtils.getOptions(this) || {}
if(query.string)
return "module.exports = `" + JSON.stringify(value) + "`;";
return "module.exports = " + JSON.stringify(value) + ";";
Copy link

Choose a reason for hiding this comment

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

Do we want to go webpack 2 here? Separate PR would be probably better for that change, though.

index.js Outdated
this.value = [value];
return "module.exports = " + JSON.stringify(value) + ";";
var value = typeof source === "string" ? JSON.parse(source) : source;
this.value = [value];
Copy link
Member

Choose a reason for hiding this comment

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

this.value = [value] can be removed like this.cacheable, not needed anymore 😛

index.js Outdated
var value = typeof source === "string" ? JSON.parse(source) : source;
this.value = [value];
var query = loaderUtils.getOptions(this) || {};
var outprefix = this.version && this.version >= 2 ? "export default " : "module.exports = ";
Copy link
Member

Choose a reason for hiding this comment

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

query => options

const options = loaderUtils.getOptions(this) || {}

const value = typeof source === "string" ? JSON.parse(source) : source

value = options.stringify ? `${JSON.stringify(value)}` : JSON.stringify(value)

const module = this.version >= 2  ? `export default ${value}` : `module.exports = ${value}` 

return module

Copy link
Contributor Author

Choose a reason for hiding this comment

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

❤️

@michael-ciniawsky
Copy link
Member

michael-ciniawsky commented Mar 15, 2017

@yuffiy Thx && sry for the nitpicking 😛

@HairyRabbit
Copy link
Contributor Author

@michael-ciniawsky
😘

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

Successfully merging this pull request may close these issues.

5 participants