search

aurelia / dialog

A dialog plugin for Aurelia.
MIT License
106 stars 115 forks source link

fix: fix umd build browser compatibility #368

Closed 3cp closed 5 years ago

3cp commented 5 years ago

closes aurelia/cli#1033

3cp commented 5 years ago

@fkleuver workflow not ready? 😄

fkleuver commented 5 years ago

Looks like this repo is not ready. Can we make develop the merge target?

3cp commented 5 years ago

Sh*t, you see what happened after I switch to develop branch. This repo's master has a big diff with v2.x.

bigopon commented 5 years ago

These following 3 fields are troublesome

  "module": "dist/es2015/aurelia-dialog.js",
  "browser": "dist/umd/aurelia-dialog.js",
  "unpkg": "dist/umd/aurelia-dialog.js",

We need an additional distribution: umd-es2015 and point unpkg to that. and we change umd distribution to umd module format + es5

fkleuver commented 5 years ago

I don't think it's worth the trouble to try and put the new workflow on this repo right now. The branches need to be stable. I'll revisit this when master is actually the latest up-to-date branch. Let's stick to just fixing the module fields on this one?

3cp commented 5 years ago

I agree. Changed back.

fkleuver commented 5 years ago

I don't know the procedures for this repo. What's the plan? Is that change in rollup config supposed to fix the whole issue or is there more stuff such as what @bigopon mentioned?

3cp commented 5 years ago

This fix for rollup is to fix browser field. The module field is a different issue.

I can put module fix in the PR together if that makes things easier.

fkleuver commented 5 years ago

Well one complete coherent PR with fix seems logical to me. That's one review, one merge, one release, then it works, right?

bigopon commented 5 years ago

Fixing rollup and issue a new build will fix the issue. An additional distribution will just be few lines of rollup config, and does not change the existing infra of this repo. Changes are:

- file: 'dist/umd/aurelia-dialog.js',
+ file: 'dist/umd-es2015/aurelia-dialog.js',
3cp commented 5 years ago

close in favour of #369.