-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
feat(gatsby): Make __experimentalThemes part of plugin APIs #15144
feat(gatsby): Make __experimentalThemes part of plugin APIs #15144
Conversation
dupe fragments in the e2es
|
|
It looks like https://github.com/gatsbyjs/gatsby/pull/15179/files means we can remove the theme logic from the query compiler, which also fixes the duplicate fragments issue here |
@@ -257,17 +257,7 @@ module.exports = async (program, directory, suppliedStage) => { | |||
rules.media(), | |||
rules.miscAssets(), | |||
] | |||
if (store.getState().themes.themes) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to keep this part of the plugin for now as I reverted full node_modules traversal from #15270.
I'll remove this piece in a follow up PR where I traverse through plugins again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to remove once #15284 is in
This change makes it so that what used to be called `__experimentalThemes` is now part of the `plugins` APIs. This means that any plugin can take advantage of the Theme APIs like Component Shadowing. * This change contains no functional changes to how themes work today. * `__experimentalThemes` and `plugins` style themes can not be used at the same time. If `__experimentalThemes` is defined, it takes precedence so as to not break existing sites. The plan here is to not break existing sites while enabling the stable version of themes to be used. We (I most likely) should go back and remove the `__experimentalThemes` handling after a suitable deprecation period. **I'd like to see this released under a tag so we can test it on a wider range of sites before merging** \# TODO - [ ] Add deprecation message for __experimentalThemes (should we do this in this PR or another?) - [ ] Should we expand the webpack handling of themes in `packages/gatsby/src/utils/webpack.config.js` to work on all plugins? I think I remember another PR
Because of gatsbyjs#14111, we can remove the custom theme processing.
159d5c6
to
79da69b
Compare
#15179 fixes the duplicated fragment issue |
Failing tests are result of duplicated plugin entries (fix in #15307 ) |
This reverts commit 81d55c3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
Published in |
This change makes it so that what used to be called
__experimentalThemes
is now part of the
plugins
APIs. This means that any plugin can takeadvantage of the Theme APIs like Component Shadowing.
__experimentalThemes
, we can change this logic to be more plugin-specific.__experimentalThemes
andplugins
style themes can not be used at thesame time. If
__experimentalThemes
is defined, it takes precedenceso as to not break existing sites.
The plan here is to not break existing sites while enabling the stable
version of themes to be used. We (I most likely) should go back and remove
the
__experimentalThemes
handling after a suitable deprecation period.I'd like to see this released under a tag so we can test it on a wider range of sites before merging
TODO
packages/gatsby/src/utils/webpack.config.js
to work on all plugins? What's the status of feat(gatsby): enable babel-loader for all dependencies #14111 in relation to this?