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

Adding -mjs.js build to support polymer-cli #88

Merged
merged 1 commit into from
Oct 19, 2018
Merged

Adding -mjs.js build to support polymer-cli #88

merged 1 commit into from
Oct 19, 2018

Conversation

cloudily
Copy link

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.309% when pulling 151b745 on cloudily:feature-polyer_cli_support_for_mjs into f67d5c2 on pladaria:master.

@pladaria
Copy link
Owner

Hi, thank you for this PR

Before accepting it I need some more context. Why do you need this?

How does polymer-cli know about the newly created artifact? It is not listed in package.json.
It is this following some kind of naming convention?

Thanks

@cloudily
Copy link
Author

cloudily commented Oct 16, 2018

Hi @pladaria,

The mjs extension has not been formalized yet. It is currently recognized by node, but not by some build tools. For example, the polymer-cli throws an error with the unrecognized mjs extension. This is to provide build tools a way to interpret these files as they relate to ES imports. Please let me know if you have any further questions.

When importing into the frontend, we use the following syntax:

import ReconnectingWebSocket from 'reconnecting-websocket/dist/reconnecting-websocket.mjs';

This currently fails build tools because of the mjs extension, hence the addition. I didn't want to change it for everyone, as it would be a breaking change.

@pladaria pladaria merged commit da027e4 into pladaria:master Oct 19, 2018
@pladaria
Copy link
Owner

All clear! Thank you for your explanation

@pladaria
Copy link
Owner

published 4.1.10 with that change

@cloudily
Copy link
Author

Thanks, @pladaria

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

Successfully merging this pull request may close these issues.

3 participants