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

Destructuring assignment of dynamically imported JSON files broken in 5.80.0 #17042

Closed
jessevanassen opened this issue Apr 21, 2023 · 14 comments · Fixed by #17418
Closed

Destructuring assignment of dynamically imported JSON files broken in 5.80.0 #17042

jessevanassen opened this issue Apr 21, 2023 · 14 comments · Fixed by #17418

Comments

@jessevanassen
Copy link

Bug report

const { default: json } = await import("./file.json");

When destructuring the default out of a dynamically imported JSON file containing an object or array, the import will result in an empty container ({} for a JSON file containing a filled object, [] for a JSON file containing a filled array) instead of the actual import.

What is the current behavior?
Only the empty container is returned, not the actual contents.

If the current behavior is a bug, please provide the steps to reproduce.
I've added the following test cases on this branch: https://github.com/jessevanassen/webpack/tree/dynamic-import-destructuring-assignment-json (comparison with main: main...jessevanassen:webpack:dynamic-import-destructuring-assignment-json)

it("should support importing objects from JSON files", async () => {
	const { default: json } = await import("./dir1/object.json"); // Contains `{ "a": 1 }`
	expect(json).toBe({ a: 1 });
});

it("should support importing arrays from JSON files", async () => {
	const { default: json } = await import("./dir1/array.json"); // Contains `["a"]`
	expect(json).toBe(["a"]);
});

it("should support importing primitives from JSON files", async () => {
	const { default: json } = await import("./dir1/primitive.json"); // Contains `"a"`
	expect(json).toBe("a");
});

Only the last test succeeds. The others fail like this:

 FAIL  test/TestCasesModule.test.js
  ● TestCases › module › chunks › destructuring-assignment › exported tests › should support importing objects from JSON files

    expect(received).toBe(expected) // Object.is equality

    - Expected  - 3
    + Received  + 1

    - Object {
    -   "a": 1,
    - }
    + Object {}

      191 | it("should support importing objects from JSON files", async () => {
      192 |     const { default: json } = await __webpack_require__.e(/*! import() */ 2).then(__webpack_require__.t.bind(__webpack_require__, /*! ./dir1/object.json */ 3, 19));
    > 193 |     expect(json).toBe({ a: 1 });
          |                  ^
      194 | });
      195 |
      196 | it("should support importing arrays from JSON files", async () => {

      at test/js/module/chunks/destructuring-assignment/bundle.mjs:193:15

  ● TestCases › module › chunks › destructuring-assignment › exported tests › should support importing arrays from JSON files

    expect(received).toBe(expected) // Object.is equality

    - Expected  - 3
    + Received  + 1

    - Array [
    -   "a",
    - ]
    + Array []

      196 | it("should support importing arrays from JSON files", async () => {
      197 |     const { default: json } = await __webpack_require__.e(/*! import() */ 3).then(__webpack_require__.t.bind(__webpack_require__, /*! ./dir1/array.json */ 4, 19));
    > 198 |     expect(json).toBe(["a"]);
          |                  ^
      199 | });
      200 |
      201 | it("should support importing primitives from JSON files", async () => {

      at test/js/module/chunks/destructuring-assignment/bundle.mjs:198:15

The following constructs do work:

const json = (await import("./file.json")).default;
const imported = await import("./file.json");
const json = imported.default;

This leads me to believe that the recently merged #16995 might be the cause.

What is the expected behavior?
The parsed JSON should be imported.

Other relevant information:
webpack version: v5.80.0
Node.js version: v20.0.0
Operating System: Ubuntu 22.04.2 LTS

@alexander-akait
Copy link
Member

/cc @vankop Can you look at this?

@alexander-akait
Copy link
Member

alexander-akait commented Apr 21, 2023

@jessevanassen Weird, I can't reproduce, can you provide example? Reproduced

@jessevanassen
Copy link
Author

For completeness, here's my reproduction.
reproduction.zip

@TheLarkInn
Copy link
Member

@jessevanassen as a work around can you get by this by dynamically importing an esm module which import the json instead? We're still looking into the fix for this.

@jessevanassen
Copy link
Author

@jessevanassen as a work around can you get by this by dynamically importing an esm module which import the json instead? We're still looking into the fix for this.

As a workaround, we rewrote it to the following which also works, but doesn't require a separate file:

const json = (await import("./file.json")).default;

It's purely the combination of dynamic imports and destructuring that triggers it, by taking out the destructuring the bug doesn't occur.

@passariello

This comment was marked as off-topic.

@passariello

This comment was marked as off-topic.

@alexander-akait
Copy link
Member

@vankop friendly ping, do you need help with it?

@vankop
Copy link
Member

vankop commented May 26, 2023

@alexander-akait hi, feel free to pick this up. dont have time for this now


basically, you need to fix how used exports are detected in this case

@alexander-akait
Copy link
Member

@vankop any hints where this is potentially being processed and needs to be fixed?)

@vankop
Copy link
Member

vankop commented May 26, 2023

FlagDependencyUsagePlugin. for me sounds like we need option in ImportDependency like importNamespace=true in this case.. since module export is exportsType=default for json module

mdmower added a commit to mdmower/doi-resolver that referenced this issue May 29, 2023
Destructuring dynamic JSON imports has been broken since webpack 5.80.0.
See webpack/webpack#17042 . Work around the
bug by avoiding the destructing pattern.
noahm added a commit to noahm/DDRCardDraw that referenced this issue May 30, 2023
@alexander-akait
Copy link
Member

alexander-akait commented Jun 10, 2023

Another intresting bug:

const { default: json, bar } = await import("./file.json");

console.log(json);
console.log(bar);

json keep bar if it exists in file.json, but should not, because there is only default

@webpack-bot
Copy link
Contributor

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.

@webpack-bot
Copy link
Contributor

Issue was closed because of inactivity.

If you think this is still a valid issue, please file a new issue with additional information.

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

Successfully merging a pull request may close this issue.

7 participants