Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

chore: build the package with another target #42

Closed
wants to merge 1 commit into from

Conversation

GuillaumeOj
Copy link
Contributor

As I'm new with many aspect of Javascript I need to be sure this really fix the issue #39 before we merged it.
Did anyone could help ?

As it was asked in #39
This add a new target for building the package.
@GuillaumeOj GuillaumeOj linked an issue Apr 6, 2021 that may be closed by this pull request
@mergify mergify bot requested a review from a team April 6, 2021 16:49
@GuillaumeOj GuillaumeOj added the help wanted Extra attention is needed label Apr 7, 2021
entry: './src/Crisp.jsx',
output: {
path: path.resolve(__dirname, './dist/web/'),
filename: 'crisp-web.js',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid, this will break update of the existing installation.

path: path.resolve(__dirname, './dist/module/'),
filename: 'crisp-module.js',
library: {
type: 'module',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why module? and not one of the 17 built-in other ways to generate a javascript file with webpack?

I'm not a huge fan of maintaining different versions of the same lib. Most frameworks support 'es' and commonjs1/2 import style. vitejs seems to have an issue with the latter, not sure we should support multiple formats just for one framework.

Copy link
Contributor Author

@GuillaumeOj GuillaumeOj Apr 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. So I close this PR.

@mergify mergify bot requested a review from a team April 7, 2021 09:14
@GuillaumeOj GuillaumeOj closed this Apr 7, 2021
@GuillaumeOj GuillaumeOj deleted the feat-export-esm branch April 7, 2021 12:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted Extra attention is needed
Development

Successfully merging this pull request may close these issues.

Issue running react-crisp with vitejs
2 participants