Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

add browser module main priority to amd processor #761

Closed
wants to merge 9 commits into from

Conversation

Toxicable
Copy link

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Feature (please, look at the "Scope of the project" section in the README.md file)

What is the current behavior?

The entry point for npm_umd_bundle in npm packages is only resolved to main

What is the new behavior?

We now try to resolve, browser, module, then main, then index.js and finally ${pkg.__name}.js

Does this PR introduce a breaking change?

Not sure, possibly

  • Yes
  • [] No

Other information

@gregmagolan
Copy link
Collaborator

I was thinking this would be the next improvement to the AMD processor. We may want to add the es6ify plugin with this change as well since some npm packages may use es6 or fesm5 in their distributions.

generate_build_file.js needs some refactoring at some point as well; it started out small and simple and has become rather large overtime.

@Toxicable Toxicable marked this pull request as ready for review May 10, 2019 22:32
@Toxicable
Copy link
Author

@gregmagolan Sounds good to me
The remaining failing test is due to not having the es6ify plugin enabled
So ill hand that over to you to handle the vendoring of that plugin when you got time for it.

@gregmagolan
Copy link
Collaborator

gregmagolan commented May 13, 2019

I took a glance at es6ify and it's a more complicated plugin than the named-amd plugin that I previously vendored in. I'll put this on my list but probably won't have time to get to it for a few weeks.

@alexeagle
Copy link
Collaborator

@Toxicable any plan how to get this green without es6ify plugin ? or should this go on the backlog?

@Toxicable
Copy link
Author

@alexeagle Definately needs the es6ify plugin for this one, since the test case I choose; rxjs with the es2015 entry point uses esm modules.
So yeah on the backlog for you guys would be the best option I think. I doubt i'd be able to vendor it correctly myself especially since @gregmagolan had a few issues with it aswell.

@gregmagolan
Copy link
Collaborator

Replaced by #997

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants