Prevent re-bundling WordPress packages#1781
Merged
westonruter merged 6 commits into1.0from Jan 3, 2019
Merged
Conversation
Collaborator
|
@westonruter Couldn't we bundle the |
westonruter
changed the title
westonruter
changed the title
Member
Author
|
@felixarntz this is ready to go! |
We should rely on ESLint alone for JS linting now. This is a follow-up to f955432
westonruter
force-pushed
the
remove/redundant-dependencies-1.0
branch
from
70f4400 to
d5dbf32
Compare
felixarntz
reviewed
Jan 2, 2019
Collaborator
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
Two minor questions, otherwise good to go.
| sourcesPointer(); | ||
| } ); | ||
| // Run at DOM ready. | ||
| jQuery( sourcesPointer ); |
Collaborator
There was a problem hiding this comment.
I might miss something, but shouldn't this use wp.domReady somehow?
Member
Author
There was a problem hiding this comment.
We could use wp.domReady but this script is already depending on jQuery so we can use jQuery ready instead which does the same thing.
westonruter
force-pushed
the
remove/redundant-dependencies-1.0
branch
from
8b6c0be to
8e2090e
Compare
Member
Author
|
Ok, issues have been addressed. |
felixarntz
approved these changes
Jan 3, 2019
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@wordpress/dom-readyand@wordpress/i18nto instead rely on existing bundles if available. Ensure both are marked asdevDependencies.@wordpress/dom-readyand@wordpress/i18nif not available (e.g. in WordPress 4.9 without Gutenberg).The@wordpress/dom-readypackage is bundled with both WordPress 5.0 and Gutenberg aswp-dom-ready, so we should use it.Also,@wordpress/i18nis not even used in the project anymore, so we don't need it at all.Problem: If on 4.9 and Gutenberg is not installed, then we need to polyfillwp-dom-readywithjQuery.ready(). Nevertheless, we are currently depending onwp-i18neven though it may not be available either. So we might need to make the plugin require WordPress 5.0 or WordPress 4.9 with Gutenberg as the minimum version. Or perhaps in that case we should still bundle those dependencies as separate scripts that we register? If we want to support 4.9 without Gutenberg, then I suppose this is what we'd have to do. @felixarntz thoughts?Closes #1763.