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

chore(amd): amd distribution #97

Merged
merged 1 commit into from
Jul 31, 2018
Merged

chore(amd): amd distribution #97

merged 1 commit into from
Jul 31, 2018

Conversation

zewa666
Copy link
Contributor

@zewa666 zewa666 commented Jul 31, 2018

the default build is done for commonjs only. Since consumers might using other bundlers, in my case requirejs - which requires amd bundles - it would be great to create another compile target.

typically a structure for multi-targets would be

lib
-> umd
-> commonjs
-> amd
-> ...

Since i didn't want to override your project structure, amd here is now compiled to dist/amd

the default build is done for `commonjs` only. Since consumers might using other bundlers, in my case requirejs - which requires amd bundles - it would be great to create another compile target.

typically  a structure for multi-targets would be

lib
  -> umd
  -> commonjs
  -> amd
  -> ...

Since i didn't want to override your project structure, amd here is now compiled to `dist/amd`
@akosyakov
Copy link
Contributor

akosyakov commented Jul 31, 2018

@zewa666 generally I am not opposed an idea. How did you know that it does what you want? I see that usually for amd everything gets bundled in one file, e.g. like monaco is bundled. Does having multiple files work for you? Do you have an example?

@zewa666
Copy link
Contributor Author

zewa666 commented Jul 31, 2018

Our example is hard to share as it involves large parts and various dependencies. AMD targets do not necessarily need to be bundled. Actually having the files seperated is desired in order to be able to lazy load single files as opposed to webpack bundles. We tested the PR here against our solution and it worked.

Copy link
Contributor

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

sounds good

@akosyakov akosyakov merged commit 71603f0 into TypeFox:master Jul 31, 2018
@zewa666
Copy link
Contributor Author

zewa666 commented Jul 31, 2018

Ideally the resulting folder structure should be changed so that lib contains subfolders per target but this ist certainly good enough for my project to keep going. Thx

@akosyakov
Copy link
Contributor

akosyakov commented Aug 1, 2018

Yes, but it will be a breaking change for downstream dependencies. I would prefer to stick to the current structure.

@akosyakov
Copy link
Contributor

@zewa666 published 0.7.1 with amd

@zewa666
Copy link
Contributor Author

zewa666 commented Aug 1, 2018

Thanks for the super fast response. We're currently in the process of updating to the new version and fixing our Service registrations, so this helps to have cleaner and easier builds without the need to run a fork

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.

None yet

2 participants