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

mangled exports breaks with destructuring assignment of JSON imports #18278

Open
ikydd-bbc opened this issue Apr 3, 2024 · 7 comments · May be fixed by #18319
Open

mangled exports breaks with destructuring assignment of JSON imports #18278

ikydd-bbc opened this issue Apr 3, 2024 · 7 comments · May be fixed by #18319

Comments

@ikydd-bbc
Copy link

ikydd-bbc commented Apr 3, 2024

Bug report

What is the current behavior?

As of 5.90.2 (including 5.91.0), destructuring assignment on a JSON import does not work because the property names have been changed (using default settings). Setting optimization.mangleExports to false fixes this, but it worked fine in 5.90.1 and it works fine accessing properties with dot notation in 5.90.2+.

Essentially the following:

import data from './data.json';

// this works ...
console.log(data.foo.bar)

// ...but this does not
const { bar } = data.foo;

If the current behavior is a bug, please provide the steps to reproduce.

https://github.com/ikydd-bbc/spike-webpack-minify-bug

What is the expected behavior?

Destructuring of JSON imports should work as they did previously in 5.90.1. We should be able to import a JSON file using the default export and then use destructuring assignment to pick out whatever properties we choose.

Other relevant information:
webpack version: 5.90.2 +
Node.js version: 18.19.0
Operating System: OSX
Additional tools:

@alexander-akait
Copy link
Member

Thank you for the issue, do you want to send a pr?

@ikydd-bbc
Copy link
Author

Thank you for the issue, do you want to send a pr?

Unfortunately I have no experience working with webpack internals or the details of project goals, so while I might be curious and have a dig to see what's what I couldn't commit to trying to fix this.

@alexander-akait
Copy link
Member

alexander-akait commented Apr 8, 2024

/cc @ahabhgk What do you think? I don't like named export with JSON at all, because it volates a spec - there is no named export with JSON files, only default (should we output a warning on it? with futureDefaults: true), maybe we should disable mangle at all for JSON modules?

@ahabhgk
Copy link
Contributor

ahabhgk commented Apr 9, 2024

I think there is already have a warning for named export with JSON

buildMeta.defaultObject =
typeof data === "object" ? "redirect-warn" : false;

What about disable named export with JSON in next major and when futureDefaults: true?

I will take a look and try to fix this bug in the next few days

@alexander-akait
Copy link
Member

@ikydd-bbc Just intresting, why you used named export for JSON? Legacy code?

@alopix
Copy link

alopix commented Apr 9, 2024

@alexander-akait
The use case shown is not really a named export from JSON though, is it?
It uses a default import and then destructures the already imported default.

import data from './data.json';

// this works ...
console.log(data.foo.bar)

// ...but this does not
const { bar } = data.foo;

@alexander-akait
Copy link
Member

@alopix Oh, I see, my mistake, sometimes the eyes are wrong 😄 We will look at this soon

@ahabhgk ahabhgk linked a pull request Apr 12, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Priority - High
Development

Successfully merging a pull request may close this issue.

5 participants