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

Systemjs depcache output #26

Merged
merged 9 commits into from
Jul 11, 2024
Merged

Systemjs depcache output #26

merged 9 commits into from
Jul 11, 2024

Conversation

mattquinlan440
Copy link
Contributor

@mattquinlan440 mattquinlan440 commented Jul 10, 2024

Thanks for opening a Pull Request

Changes

Please outline the reason for the changes you are making:

This PR adds the ability to output the scripts in a depcache format that's usable with single SPA / systemjs implementations

[Change description here]

Are there any breaking changes you are aware of in the PR?

Not directly. Updating arborist a major version, but its downstream change removed node versions we already didn't support.

General Checklist

  • Change is tested locally
  • Demo is updated to exercise change (if applicable)
  • [WIP] flag is removed from the title

@mattquinlan440 mattquinlan440 marked this pull request as ready for review July 10, 2024 20:06
@mattquinlan440 mattquinlan440 requested a review from a team as a code owner July 10, 2024 20:06
.help('-h, --help')
.parseAndExit()
.then(options => ({ ...options, logging: loggingMap[options.logging] }))
.then(options => new CliOptions(options))
.then(cliOptions => buildAppOutput(cliOptions))
.then(cliOptions => {
if(cliOptions.outputDepcache) {
Copy link
Member

Choose a reason for hiding this comment

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

Could we get a description and/or sample in the README of what this is and does?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added something to the README. Let me know if it's enough, or more details are needed

Copy link
Member

Choose a reason for hiding this comment

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

That does the job nicely. Thank you

Copy link
Member

@jtheriault jtheriault left a comment

Choose a reason for hiding this comment

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

This is a good extension and also a good application of "1, 2, n." I think there's some opportunity identifying some common abstractions when we add an "nth" output type, for example import maps.

@mattquinlan440 mattquinlan440 merged commit 392d5fb into main Jul 11, 2024
3 checks passed
@mattquinlan440 mattquinlan440 deleted the systemjs-depcache-output branch July 11, 2024 13:38
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.

4 participants