Skip to content

Default to new CopyStaticAssetsPlugin, and provide flag to revert back to old plugin #45

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jun 28, 2021
Merged

Default to new CopyStaticAssetsPlugin, and provide flag to revert back to old plugin #45

merged 5 commits into from
Jun 28, 2021

Conversation

calebmshafer
Copy link
Member

@calebmshafer calebmshafer commented Jun 28, 2021 •
edited
Loading

@aruniverse let me know what you think of the flag and description. I was debating turning it on by default instead of requiring a flag. That way any app that's not using workspaces can get the advantages and we can just explicitly turn it off in the other repos...

Once we finish this PR, I'll port to the 4.x PR.

@calebmshafer calebmshafer requested a review from aruniverse June 28, 2021 02:09
aruniverse
aruniverse reviewed Jun 28, 2021
View reviewed changes
Copy link
Member

@aruniverse aruniverse left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This is a good alternative, could you bump the pkg ver too? I don't really have a prefernce for which one should be enabled by default, not sure how many teams are using pnpm workspaces, you might have a better idea than me

@aruniverse
Copy link
Member

Actually thinking about it now, it's probably more preferable to default to the new plugin, and use a flag to use the old one when you're using workspaces.
Just so when users bootstrap a new app, strings will be localized and look a bit more professional right off the bat.

@calebmshafer
Copy link
Member Author

Actually thinking about it now, it's probably more preferable to default to the new plugin, and use a flag to use the old one when you're using workspaces.
Just so when users bootstrap a new app, strings will be localized and look a bit more professional right off the bat.

@aruniverse agreed, I'll flip it around.

aruniverse
aruniverse reviewed Jun 28, 2021
View reviewed changes
aruniverse
aruniverse reviewed Jun 28, 2021
View reviewed changes
@aruniverse aruniverse changed the title Revert the default usage of the new asset copying workflow behind a flag Default to new CopyStaticAsstesPlugin, and provide flag to revert back to old plugin Jun 28, 2021
@aruniverse aruniverse changed the title Default to new CopyStaticAsstesPlugin, and provide flag to revert back to old plugin Default to new CopyStaticAssetsPlugin, and provide flag to revert back to old plugin Jun 28, 2021
@calebmshafer calebmshafer merged commit ca41989 into 3.x Jun 28, 2021
@calebmshafer calebmshafer deleted the revert-new-asset-copy branch June 28, 2021 13:36
aruniverse pushed a commit that referenced this pull request Jun 28, 2021
@calebmshafer @aruniverse
Default to new CopyStaticAssetsPlugin, and provide flag to revert bac…
45b126c 
…k to old plugin. 3.4.13 (#45)

* Add a flag called DISABLE_NEW_ASSET_COPY to turn off new asset copying plugin

* Bump version 3.4.13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aruniverse aruniverse aruniverse approved these changes

@wgoehrig wgoehrig wgoehrig approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@calebmshafer @aruniverse @wgoehrig