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

Avoid importing .css files in the published files #2561

Closed
abdonrd opened this issue Jul 21, 2021 · 4 comments
Closed

Avoid importing .css files in the published files #2561

abdonrd opened this issue Jul 21, 2021 · 4 comments
Assignees
Labels
Type: Bug Something isn't working

Comments

@abdonrd
Copy link
Contributor

abdonrd commented Jul 21, 2021

Describe the bug

https://unpkg.com/browse/@material/[email protected]/mwc-button.js

I just checking the @material/mwc-button and it has this CSS import line:

import { styles } from './styles.css';

But the browser can't load this .css file (at least without server magic).

Expected behavior

Use code that the browser understand.

Maybe as .css.js?

import { styles } from './styles.css.js';
@abdonrd abdonrd added Focus Area: Components Type: Bug Something isn't working labels Jul 21, 2021
@dfreedm
Copy link
Collaborator

dfreedm commented Jul 22, 2021

I think for both this reason, and future support of Import Maps, we'll have to evaluate adding .js file extensions to our built files.

Our internal styleguide for Typescript precludes adding using file extensions on source files, so we'll probably look first at using some tooling in the build process to add file extensions.

@thescientist13
Copy link

thescientist13 commented Aug 2, 2021

Our internal styleguide for Typescript precludes adding using file extensions on source files, so we'll probably look first at using some tooling in the build process to add file extensions.

This would be great to see! Started testing out MWC myself and also ran into this.

Also and apologies if I am missing it, but I don't actually see that styles.css in the published package. Is that expected? 🔍 👀

Screen Shot 2021-08-02 at 5 04 32 PM


On a related note, would this effort also address the missing .js extension as well from files also referenced in the entry point? Like mwc-button-base? I'm think that per the spec that files need to have an extension, so would / could this be part of a related effort to address extensions within the project?


Not sure if any of these points are worth raising on the README but happy to submit a PR to help highlight any of these considerations and / or making an issue to track.

Thanks! ✌️

@thescientist13
Copy link

thescientist13 commented Aug 4, 2021

Ah, after thinking about it some more, it seems that the assumption is that @material/mwc-button/styles.css will be resolved to @material/mwc-button/styles.css.js presumably by checking / adding the .js at the consumer level? Sorry if I missed all this in the docs. 😞

Should be easy enough to work around but I think per spec compliance, and to favor being explicit over implicit, it would be great if MWC could always set the extension of the file being imported. 🙏

@dfreedm
Copy link
Collaborator

dfreedm commented Aug 20, 2021

Merging this issue into #2632

@dfreedm dfreedm closed this as completed Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants