Skip to content

Load word-count and do not allow our wp-utils script to clobber it#1571

Merged
nylen merged 6 commits into
WordPress:masterfrom
ryanwelcher:fix/1569
Jun 29, 2017
Merged

Load word-count and do not allow our wp-utils script to clobber it#1571
nylen merged 6 commits into
WordPress:masterfrom
ryanwelcher:fix/1569

Conversation

@ryanwelcher

Copy link
Copy Markdown
Contributor

Renaming the utils to utils-gb to avoid overidding the `wp.utils' object. Fixes #1569

@nylen

nylen commented Jun 28, 2017

Copy link
Copy Markdown
Member

There is a cleaner way to fix this, especially given that these two things will probably eventually be merged into wp.utils together (see ongoing conversation about JS modules in core). We just need to require that the webpack version of wp.utils is loaded first. From previous discussion around #940 (comment):

word-count.js doesn't clobber window.wp.utils

So this means that the webpack version needs to be loaded first. We can accomplish this by adding wp-utils as a dependency of word-count.

Also I think it's a bit weird that as a result of this PR, our utilities would be accessed via window.wp['utils-gb'] in externall code.

@ryanwelcher

Copy link
Copy Markdown
Contributor Author

Until everything that lives in wp.utils is merged into the GB version, any item that lives in that object will not be available when Gutenberg is loaded. That will mean that wp-utils will have to be a dependency for anything that core adds to the wp.utils object and then only in the context of Gutenberg so the script will load.

@swissspidy swissspidy changed the title Fix/1569 Rename utils to utils-gb Jun 28, 2017
@adamsilverstein

Copy link
Copy Markdown
Member

@nylen any suggestions for how we "add wp-utils as a dependency of word-count."? Do you mean in the webpack config?

@nylen nylen force-pushed the fix/1569 branch 2 times, most recently from 68f6755 to f43c8e9 Compare June 28, 2017 21:38
@nylen

nylen commented Jun 28, 2017

Copy link
Copy Markdown
Member

any suggestions for how we "add wp-utils as a dependency of word-count."?

As done in f43c8e9. To test, load the plugin and verify that wp.utils.WordCounter (from core) and wp.utils.keycodes (from wp-utils) are both available.

That will mean that wp-utils will have to be a dependency for anything that core adds to the wp.utils object and then only in the context of Gutenberg so the script will load.

It doesn't quite mean that, because we will only add the word-count -> wp-utils dependency when we are actually loading wp-utils in Gutenberg context.

As discussed in Slack, I expect the longer-term way to resolve this will be to make wp.utils the first module that Webpack builds for us in WP core. It's a great candidate since the core version only contains this one thing.

@nylen nylen changed the title Rename utils to utils-gb Load word-count and do not allow our wp-utils script to clobber it Jun 28, 2017
@nylen

nylen commented Jun 28, 2017

Copy link
Copy Markdown
Member

This was not in the right place the first time around. Amended in b41ff21.

@ryanwelcher

Copy link
Copy Markdown
Contributor Author

I've reverted the changes to the folder names. Thanks for your insight and help on this @nylen !

@nylen nylen merged commit 8562c16 into WordPress:master Jun 29, 2017
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.

3 participants