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

JSON and CSS modules dynamic import #835

Closed
dandclark opened this issue Sep 9, 2019 · 5 comments
Closed

JSON and CSS modules dynamic import #835

dandclark opened this issue Sep 9, 2019 · 5 comments

Comments

@dandclark
Copy link
Contributor

dandclark commented Sep 9, 2019

When preparing the initial implementation of JSON modules in Blink, we noticed a potentially interesting issue with dynamic import.

The original JSON module WPT test for dynamic import expected this behavior:

const result = await import(`./resource.json`);
assert_equals(result, value); // where 'value' is the same as the JSON object in resource.json

However, given the existing behavior of Synthetic Modules with dynamic import(), result actually ends up being the namespace object for the imported module, and the importer needs to check the default property to get the actual value:

const result = await import(`./resource.json`);
assert_equals(result.default, value); // where 'value' is the JSON object in resource.json

One can argue that this was just a bug in the initial test, and we did in fact end up changing the test as part of submitting the Blink code change.

It doesn't quite seem ideal ideal to make the importer dig into the namespace object's default property, but it is consistent with the existing semantics for default exports. I'm not sure whether it's a serious enough problem to consider changing the semantics of how these work. A solution would either require reimplementing JSON modules without Synthetic Modules, or extending Synthetic Modules in some way, perhaps with an addition of a custom FinishModuleImport-like callback where the implementer of the Synthetic Module can customize what gets returned from an import() of the module.

All of the above applies to CSS modules in the same way, as these too are built as a Synthetic Module with a single default export of the stylesheet.

Thoughts?
cc: @littledan @Ms2ger @travisleithead @BoCupp @samsebree @domenic @justinfagnani

@domenic
Copy link
Collaborator

domenic commented Sep 10, 2019

This is totally reasonable and expected from my perspective. I don't think there's any other possible semantics for combining the JS module system and its default export concept.

@matthewp
Copy link

I agree with the above, this is expected given the semantics of modules. You also can't do import { deep } from './resource.json';. This is actually a little nicer with dynamic import because at least you can destruct there: const { deep } = (await import('./resource.json')).default;

@justinfagnani
Copy link
Contributor

justinfagnani commented Sep 10, 2019

Agreed with @domenic and @matthewp as well as this:

All of the above applies to CSS modules in the same way, as these too are built as a Synthetic Module with a single default export of the stylesheet.

For CSS modules a single default export is potentially only a starting point. I have a follow-on proposal to allow for exports from a CSS module: w3c/csswg-drafts#3714 For that we would need dynamic import to work as it does with JS modules.

@dandclark
Copy link
Contributor Author

Thanks all -- I'll consider the issue closed as there seems to be a pretty clear agreement on the direction here.

@Ms2ger
Copy link

Ms2ger commented Sep 16, 2019

The initial expectation of the test case was indeed just a mistake on my side. Thanks for following up!

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

No branches or pull requests

5 participants