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

Use get_plugin_data for production script versioning (cachebust) #440

Closed
aduth opened this issue Apr 17, 2017 · 18 comments
Closed

Use get_plugin_data for production script versioning (cachebust) #440

aduth opened this issue Apr 17, 2017 · 18 comments
Assignees
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript Good First Issue An issue that's suitable for someone looking to contribute for the first time [Status] Needs More Info Follow-up required in order to be actionable. [Type] Task Issues or PRs that have been broken down into an individual action to take

Comments

@aduth
Copy link
Member

aduth commented Apr 17, 2017

(Edit 2017-04-27): Basic cachebusting exists with #509 and #491 using filemtime. This works well for development environments, but we'll want something more reliable for production builds; likely the plugin version, extracted using get_plugin_data.


We don't currently version any of our scripts (source). It can be misleading to navigate to the editor and not have the latest script changes applied due to caching. While developing, it's easiest to have browser developer tools open with cache disabled (Chrome example). This is easy to overlook and as we transition to a published plugin, we should avoid this requirement.

Instead, we ought to version scripts. This will likely correspond to tagged versions we release, currently fixed at 0.1.0 .

A few questions:

  • How do we maintain this version? Should the plugin file read from package.json or maintain its own internal constant?
  • Could we get more granular with versioning by git HEAD SHA, assuming the repository has been cloned?
  • We could include a hash in the generated script filename and write these to a plugin-accessible file during build (documentation)
@aduth aduth added Good First Issue An issue that's suitable for someone looking to contribute for the first time Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Task Issues or PRs that have been broken down into an individual action to take labels Apr 17, 2017
@kopepasah
Copy link
Member

How do we maintain this version? Should the plugin file read from package.json or maintain its own internal constant?

If we were to use a version of the plugin, I would recommend using the plugin version using get_plugin_data(), (which could be switched to the WordPress version once merged into Core).

Could we get more granular with versioning by git HEAD SHA, assuming the repository has been cloned?

While this makes sense for development, it will not be reliable for users which may download the plugin and install. While this may not be happening now (even though it is possible), it could happen in the future.

We could include a hash in the generated script filename and write these to a plugin-accessible file during build.

While this may work, PHP provides methods of cache busting without using information from within the file.

Since we are only enqueuing built scripts, I would recommend using the first option (plugin version) for production and filemtime() during development (or even when WP_DEBUG is true).

Once a direction is decided, I would like to tackle this issue.

@nylen
Copy link
Member

nylen commented Apr 19, 2017

@kopepasah your proposed direction sounds good to me:

  • Use filemtime during development / WP_DEBUG
  • Use plugin version during production

I don't think both of these need to be done in the same PR either, starting with development would be fine as we are "pre-alpha" with no released versions yet.

@aduth
Copy link
Member Author

aduth commented Apr 19, 2017

I definitely overlooked get_plugin_data and filemtime. Those sound like great options for releases and development respectively. Thanks for taking this on @kopepasah .

@kopepasah
Copy link
Member

@nylen sounds good to me.

I know this ticket cannot be assigned to me, but perhaps we can have some indication that is currently under development?

@aduth aduth added the [Status] In Progress Tracking issues with work in progress label Apr 19, 2017
@aduth
Copy link
Member Author

aduth commented Apr 19, 2017

I created and assigned an "In Progress" label to the issue. "Assigned" might have been a more accurate name for the current need, but thought this could work well to share for pull requests that aren't quite complete.

@kopepasah
Copy link
Member

Created a pull request to address this issue. I kept it as simple as possible, as these scripts and styles will probably be handed by Core in the future.

@BE-Webdesign
Copy link
Contributor

Pretty sure this was completed in #509, and #491. Reopen if I goofed up.

@aduth
Copy link
Member Author

aduth commented Apr 27, 2017

I'm going to reopen but clarify the title and text of the original comment. You're right that #509 and #491 introduce cachebusting, but I don't think we'll want to use filemtime for production releases. Per discussion here sounds as though we've settled on get_plugin_data outside the WP_DEBUG context.

@aduth aduth changed the title Add script versioning (cachebust) Use get_plugin_data for production script versioning (cachebust) Apr 27, 2017
@kopepasah
Copy link
Member

@aduth should we create a different issue for this instead of reopening this one? I believe there would be a little more to it than just adding get_plugin_data, as currently we do not have any scripts or styles committed to the repo.

@aduth
Copy link
Member Author

aduth commented Apr 28, 2017

It depends if we feel strongly about changing the behavior of the production output. I don't think the plan is to include the compiled files in the repository, but instead in the attachment of a release, using the default behavior of npm run build which will output minified files at the same */build/index.js locations. If that's the route we took, it could be as simple as using the release version instead of filemtime.

The potential downsides being:

  • Maybe we don't want to include the source files in the production build
  • We might rather use a .min.js extension to indicate the files are minified
  • It's still not easy to run the repository directly cloned from GitHub

Also, just noting now that get_plugin_data notes that it's only available in the wp-admin, meaning if we'd want to use it, we must enqueue it on admin screens only, limiting any potential use of the editor on the front-end.

@aduth aduth reopened this Apr 28, 2017
@mtias mtias removed the [Status] In Progress Tracking issues with work in progress label Apr 28, 2017
@nylen
Copy link
Member

nylen commented Apr 28, 2017

I don't think the plan is to include the compiled files in the repository, but instead in the attachment of a release

+1 to this, including built files in a repository is a misuse of source control.

  • Maybe we don't want to include the source files in the production build
  • We might rather use a .min.js extension to indicate the files are minified

+1 to both of these points as well.

  • It's still not easy to run the repository directly cloned from GitHub

I think it's fine to address this separately with better documentation and other incremental improvements.

@kopepasah kopepasah self-assigned this May 1, 2017
@kopepasah
Copy link
Member

  • Maybe we don't want to include the source files in the production build
  • We might rather use a .min.js extension to indicate the files are minified

I also think these are good idea, but the .min.js extension should not prevent us from moving forward and changing later when the package release process is implemented.

we must enqueue it on admin screens only, limiting any potential use of the editor on the front-end.

This is true only if we intend to allow public facing editing outside of the customizer; however, the new editor paired with the Customizer would be an extremely powerful feature.


One final consideration is should we implement the filemtime with WP_DEBUG or SCRIPT_DEBUG/STYLE_DEBUG?

@kopepasah
Copy link
Member

I've added a helper function for determining which type of version to use for cache busting here: #585

@danielbachhuber
Copy link
Member

What's remaining on this issue?

@nylen
Copy link
Member

nylen commented Jan 26, 2018

Currently we are including the plugin version in the built plugin zip, which makes get_plugin_data unnecessary: https://github.com/WordPress/gutenberg/blob/v2.1.0/bin/generate-gutenberg-php.php

I think this can probably be closed.

@aduth
Copy link
Member Author

aduth commented Jan 30, 2018

I don't think we're using that plugin version though. On a test site running the production Gutenberg release plugin, I'm still seeing timestamps in the ?ver query parameter.

So I still see a remaining action item here: Use the plugin version for production release script loading.

@danielbachhuber
Copy link
Member

My suggestion would be to introduce a GUTENBERG_VERSION constant. The constant's value could be manually updated on release, or the build process can write a VERSION plain text file and the constant value can be set from the file. This is how WP-CLI does it: https://github.com/wp-cli/wp-cli/blob/851a650d8518d94acc91b17ee098f3cfcbfeabd3/php/wp-cli.php#L5

@aduth aduth closed this as completed Feb 8, 2018
@aduth
Copy link
Member Author

aduth commented Feb 8, 2018

The changes requested here would only save against unnecessary cache busts from use of timestamp. As it's been nearly a year since the issue was opened and it remains a low priority to resolve, I'm closing it.

It could still be a worthwhile enhancement, though low priority, so if someone wants to take this on, I'd be happy to reopen and reassign.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript Good First Issue An issue that's suitable for someone looking to contribute for the first time [Status] Needs More Info Follow-up required in order to be actionable. [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

No branches or pull requests

7 participants