Webpack5 JSON module import with Object destructuring assignment should drop unused JSON properties #11676
Labels
Comments
dr-js
added a commit
to dr-js/dr-run
that referenced
this issue
Oct 14, 2020
notable change: - break: use `@dr-js/*@0.4.0-dev*` - break: change to new server|module pattern - add: feature: Weblog with Markdown - add: use `ActionJSON` to trigger generate Weblog - add: ci: use GitHub Action - todo: wait: terser/terser#851 - todo: check: webpack/webpack#11676 - script sort - package update
|
@dr-js the warning only appears for // commonjs, destructuring
const { foo: foobar } = require('./bar')// es6 module, named import
import { foo as foobar } from './bar' |
|
additional information: #9246 to give you some background on the warning: how do you import the following import { default as foo} from './foo.json'
// which is the equivalent for:
import foo from './foo.json'{
"default": "foo"
}the other problem is that not everything is an "object". how do you import the following with named imports? [0,1,2,3,4]or |
|
@dnalborczyk thanks for expanding the explanation, and I'm with you on locking JSON to The feature request is about support drop unused JSON properties when the Object destructuring assignment is used on the imported JSON Object. Your point on fallback to using the |
I'm not sure if that would be even feasible, maybe in only really simple cases? or the structure might be re-exported:
sorry, that wasn't meant as a fallback suggestion. I don't know if webpack does any treeshaking in that case either. that was just to line out the difference between destructuring and named imports. there are more differences than just the syntax. I think it's best explained here, which I think also applies as a case against tree shaking: |
|
I've added 2 more test, and the bad news is currently using For the record, currently my only JSON import usage is for getting |
// FILE: `test-1-default-import-basic-assignment.js`
// BUILD: no warning
// OUTPUT: no unused JSON properties: `const r=JSON.parse('{"u2":"test-webpack5-json-module","i8":"0.0.0"}')`
import PACKAGE_JSON from './package.json'
const packageName = PACKAGE_JSON.name
const packageVersion = PACKAGE_JSON.version
console.log({ packageName, packageVersion })This works. Destructuring will eventually supported too, but we currently paused feature development |
|
Yeah, that's the code style I'm using, since I prefer to limit the code dependency at top of the file. It's good to know this feature is coming, and thank you all for the hard work! |
|
Hate to be that guy, but ... Have you considered to just use a custom loader keyed on |
|
@rjgotten I think you're right, for my usage this is the exact need. |
|
The loader const EXPORT_MODE_EXPORT_EACH = 'export-each' // export like: `export const ${key} = ${JSON.stringify(value)}`, need set `type: 'javascript/auto'`, check: https://webpack.js.org/configuration/module/#ruletype
const EXPORT_MODE_EXPORT_DEFAULT = 'export-default' // export like: `export default { ${key0}, ${key1}, ${key2}, ... }`, need set `type: 'javascript/auto'`
const EXPORT_MODE_EXPORT_BOTH = 'export-both' // export both named and default, need set `type: 'javascript/auto'`
const EXPORT_MODE_JSON = 'json' // export as picked JSON Object String
const EXPORT_MODE_LIST = [ EXPORT_MODE_EXPORT_EACH, EXPORT_MODE_EXPORT_DEFAULT, EXPORT_MODE_EXPORT_BOTH, EXPORT_MODE_JSON ]
const JSONPickLoader = function (sourceString) {
const sourceObject = JSON.parse(sourceString)
if (!isBasicObject(sourceObject)) throw new Error(`[JSONPickLoader] source file should be Object JSON, got: ${String(sourceObject)}`)
const { query: options } = this // https://webpack.js.org/api/loaders/#thisquery
if (!isBasicObject(options)) throw new Error(`[JSONPickLoader] only JSON option supported, got: ${String(options)}`) // https://github.com/webpack/loader-utils/blob/v2.0.0/lib/getOptions.js#L12-L15
const { keys = [], exportMode = EXPORT_MODE_EXPORT_BOTH } = options // NOTE: names in `options.keys` should be valid JS variable names.
if (!EXPORT_MODE_LIST.includes(exportMode)) throw new Error(`[JSONPickLoader] invalid exportMode: ${String(exportMode)}`)
const outputStringList = []
switch (exportMode) {
case EXPORT_MODE_EXPORT_EACH: {
for (const key of keys) {
const value = sourceObject[ key ]
verifyPick(key, value)
outputStringList.push(`export const ${key} = ${JSON.stringify(value)}`)
}
break
}
case EXPORT_MODE_EXPORT_DEFAULT:
case EXPORT_MODE_EXPORT_BOTH: {
const isBothMode = exportMode === EXPORT_MODE_EXPORT_BOTH
const exportItemList = []
for (let index = 0, indexMax = keys.length; index < indexMax; index++) {
const key = keys[ index ]
const value = sourceObject[ key ]
verifyPick(key, value)
outputStringList.push(isBothMode ? `export const ${key} = ${JSON.stringify(value)}` : `const _${index} = ${JSON.stringify(value)}`)
exportItemList.push(isBothMode ? key : `_${index} as ${key}`)
}
outputStringList.push('export default {', exportItemList.join(','), '}')
break
}
case EXPORT_MODE_JSON: {
const pickedObject = {}
for (const key of keys) {
const value = sourceObject[ key ]
verifyPick(key, value)
pickedObject[ key ] = value
}
outputStringList.push(JSON.stringify(pickedObject))
break
}
default:
throw new Error(`[JSONPickLoader] invalid exportMode: ${String(exportMode)}`)
}
return outputStringList.join('\n')
}
const isBasicObject = (value) => (typeof value === 'object' && value !== null && !Array.isArray(value))
const verifyPick = (key, value) => { if (value === undefined) throw new Error(`[JSONPickLoader] source JSON missing key: ${String(key)}`) }
module.exports = JSONPickLoaderThe sample usage: config.module = { rules: [ {
test: /package\.json$/,
type: 'javascript/auto',
use: { exportMode: 'export-both', loader: path.join(__dirname, 'webpack-json-pick-loader.js'), options: { keys: [ 'name', 'version' ] } }
} ] }
config.module = { rules: [ {
test: /package\.json$/,
use: { exportMode: 'json', loader: path.join(__dirname, 'webpack-json-pick-loader.js'), options: { keys: [ 'name', 'version' ] } }
} ] }The updated test pack with loader And test log from [output-test.log]From current test, seems it's better to match |
dr-js
added a commit
to dr-js/dr-dev
that referenced
this issue
Oct 27, 2020
notable change: - add: `webpack-json-pick-loader` to filter `package.json` explicitly, related: webpack/webpack#11676 - fix: ci: enable color on GitHub Actions - fix: `npxLazy` re-run and test - script sort - package update
dr-js
added a commit
to dr-js/dr-dev
that referenced
this issue
Oct 29, 2020
notable change: - break: use `@dr-js/*@0.4.0` - break: use `webpack@5` - break: use async `minifyFileWithTerser` instead of sync `minifyWithTerser` from `minify.js` - break: limit eslint browser globals to window only - add: `webpack-json-pick-loader` to filter `package.json` explicitly, related: webpack/webpack#11676 - add: `isPublishVerify` to `publishOutput` - add: support `publish-auto` - fix: `npxLazy` re-run and test - fix: prevent clear whole pwd when omitting `fromOutput` of `clearOutput` - fix: ci: enable color on GitHub Actions - adjust `npxLazy` for `npm@7`: npm/cli@5473bbda - enable new JSX transform: https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html - script sort - package update
|
This issue had no activity for at least three months. It's subject to automatic issue closing if there is no activity in the next 15 days. |
|
bump |
|
Issue was closed because of inactivity. If you think this is still a valid issue, please file a new issue with additional information. |
|
Closing in favor of #14800 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Feature request
Webpack5 will emit a warning when a non-default export is used, but using the default JSON export may cause the whole file being packed, depending on the type of assignment being used.
The whole test setup: test-webpack5-json-module-20201017.zip (with
webpack@5.1.3)The test can be run with
npm i && npm tafter unzip.[Sample log of one test run]
What is the expected behavior?
When using Object destructuring assignment and the JSON module the output can drop unused JSON properties, like with normal assignment.
What is motivation or use case for adding/changing the behavior?
To provide more coding style choice, and avoid syntax trap causing output bloat and potential info leak, since one common JSON import source is
package.json, and people put all things inside.How should this be implemented in your opinion?
Are you willing to work on this yourself?
Sorry, currently I have no idea how webpack internal works.
The text was updated successfully, but these errors were encountered: