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

Build Tooling: Avoid `.default` on browser global assignments #12006

Merged
merged 2 commits into from Nov 19, 2018

Conversation

Projects
None yet
3 participants
@aduth
Member

aduth commented Nov 16, 2018

Related: #11825 (comment)

This pull request seeks to correct wrongly-assigned .default values on browser-assigned globals. In master you will observe both a wp.tokenList.default and a wp.coreData.default.

This was discovered with the following console code:

Object.entries( wp ).filter( ( [ key, value ] ) => value.default )

Open question: These are technically breaking changes, though very much of the "this was obviously always broken" sort. We don't use core-data's default export anywhere, nor should anyone. Should we have any deprecation process here?

Testing instructions:

Verify that wp.tokenList is the default exported function.

Verify there is no wp.coreData.

Follow-up tasks:

As noted at #11825 (comment) and similarly considered in Slack (link requires registration), we should consider correcting the naming of tokenList and escapeHtml.

@gziolo

gziolo approved these changes Nov 19, 2018

I'm fine with the changes proposed. I don't think that someone would use wp.tokenList.default in their code. It's a very low-level utility object. We should fix it now rather than support indefinitely for backward compatibility.

@gziolo gziolo added this to the 4.5 milestone Nov 19, 2018

@@ -167,6 +167,7 @@ const config = {
'deprecated',
'dom-ready',
'redux-routine',
'token-list',

This comment has been minimized.

@youknowriad

youknowriad Nov 19, 2018

Contributor

We should not forget to update this in Core's webpack too :)

@youknowriad youknowriad merged commit 08fe16a into master Nov 19, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@youknowriad youknowriad deleted the fix/token-list-default branch Nov 19, 2018

grey-rsi pushed a commit to OnTheGoSystems/gutenberg that referenced this pull request Nov 22, 2018

Build Tooling: Avoid `.default` on browser global assignments (WordPr…
…ess#12006)

* Core Data: Avoid exporting default

* Token List: Assign exported default to global
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment