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

Update sharetribe-scripts v5.0.1 -> v6.0.0 #1531

Merged
merged 105 commits into from
Jul 11, 2022
Merged

Conversation

Gnito
Copy link
Contributor

@Gnito Gnito commented Jul 7, 2022

NOTE: THIS IS GOING TO CAUSE A MAJOR VERSION CHANGE TO TEMPLATES.

This PR updates sharetribe-scripts to the next major version.

sharetribe-scripts is a fork of Create React App (CRA) repository and specifically react-scripts is modified to support server-side rendering, code-splitting, and certain PostCSS plugins that we chose to include years ago. (E.g. introduced CSS Modules before CRA took the same approach.)

This update is actually an update from CRA 4.0.2 to v5.0.1

The breaking changes are in v5.0.0

Highlights from CRA update:

Note: we haven't explored Tailwind support since it would mean a complete rewrite for the FTW templates. In addition, CSS Modules make it easier for customizers to learn a big codebase like FTW (i.e. which component is responsible for a different sections of DOM, etc.).


Consequences

Major updates to Webpack (v4 > v5) and PostCSS (v7 > v8), were the most worksome for FTW templates.

  • The upper restriction of Node.js engine "version earlier than v17" has been lifted!

  • Loadable components was updated

    • FTW and other client apps should update their part of Loadable components too
      • "@loadable/component": "^5.15.2",
      • "@loadable/server": "^5.15.2",
  • You should add @importfor customMediaQueries.css file.

    • E.g. @import '../../styles/customMediaQueries.css';
    • Some parallelism has increased in the build process and postcss-import wasn't included without it being explicitly mentioned per CSS file.
  • src/index.js was refactored.

    • We rearranged the module imports
      • marketplaceDefaults.css was introduced first before any of the components were imported.
      • This ensures that the content of marketplaceDefaults.css is first in the built CSS file (main chunk).
        (The order of classes in CSS files affects declaration overrides.)
  • postcss-apply plugin is not working anymore

    • It's been deprecated for a long time because W3C decided to not go forward with that feature suggestion

    • You should migrate away from postcss-apply syntax (aka CSS Property Sets)

      • Most of the code changes in this PR are about migrating away from CSS Property Sets
    • However, we have introduced a naive custom version of postcss-apply as a private plugin to give more time to migrate away.

      Most of the previous CSS Property Sets are turned into CSS classes in the marketplaceDefaults.css file.
      So, most of the time the change is minimal:

      .author {
        @apply --marketplaceH4FontStyles;
      }
      
      .author {
        composes: h4 from global;
      }
      

      Unfortunately, there's one problem that doesn't make it that straightforward. The order of the plain CSS classes in the marketplaceDefaults.css file defines which classes override each other. (The composes feature just includes those class names to DOM element - not their content.) Therefore, if multiple @apply rules are used inside the CSS module class, you need to check if there are overwrites between declarations of those Sets.

      Button styling included almost always some overrides. We decided to split buttons styles into smaller chunks that can be combined:

      .buttonLink {
        @apply --marketplaceButtonStyles;
      }
      .buttonLinkPrimary {
        @apply --marketplaceButtonStylesPrimary;
      }
      
      .buttonLinkSecondary {
        @apply --marketplaceButtonStylesSecondary;
      }
      
      

      New syntax:

      .buttonLink {
        composes: button buttonFont buttonText buttonBorders buttonColors from global;
      }
      .buttonLinkPrimary {
        composes: button buttonFont buttonText buttonBorders buttonColorsPrimary from global;
      }
      
      .buttonLinkSecondary {
        composes: button buttonFont buttonText buttonBordersSecondary buttonColorsSecondary from global;
      }
      

      This might be a bit controversial, but it introduces fewer CSS overrides. Of course, you are free to customize this to your liking.


Potential issues that you might encounter if taking an update from upstream

NOTE: You may need to delete your node_modules folder and reinstall your dependencies by running npm install (or yarn) if you encounter errors after upgrading.

NOTE 2: If you have moved components under page-level containers (like in FTW-product), those components are likely to be nested more deeply. You might need to adjust relative paths to the customMediaQueries.css file.

Old version of @babel/runtime

Module not found: Error: Can't resolve './defineProperty' in '/Users/vesaluusua/st/ftw-daily/node_modules/redux/node_modules/@babel/runtime/helpers/esm'
Did you mean 'defineProperty.js'?

Redux has sub-dependency to @babel/runtime. An old version of that package doesn't handle ES Modules well. You might need to call yarn upgrade redux after yarn install to update Redux sub dependencies to the latest in yarn.lock file.
reduxjs/redux#4174

customMediaQueries.css was not @imported in the file that uses them

You should be able to find correct files by grepping files that don't contain "customMediaQueries" but which contain "--viewport".

In OSX, that could be done in Terminal with:

grep -r --include \*.module.css -L customMediaQueries ./src | xargs grep -l '\-\-viewport'

Missing propertySets.css file and your custom CSS Property Sets

You should consider if you want to refactor your custom @apply rules (i.e. migrate away as recommended) or reintroduce / revert the deletion of the propertySets.css file.

Some changes related to naming:

  • button styles as described before.

  • --marketplaceLinkStyles > .a

  • --marketplaceH1FontStyles > .h1

  • --marketplaceH2FontStyles > .h2

  • --marketplaceH3FontStyles > .h3

  • --marketplaceH4FontStyles > .h4

  • --marketplaceH5FontStyles > .h5

  • --marketplaceH6FontStyles > .h6

  • marketplaceTabNavFontStyles was inlined for FTW-daily

  • marketplaceTabNavHorizontalFontStyles was inlined for FTW-daily

  • marketplaceSmallFontStyles was inlined for FTW-daily and FTW-hourly but not for FTW-product

Wrong syntax for CSS Property Sets (postcss-apply)

Some of the definitions of CSS Property Sets in the FTW codebase used a wrong syntax to define them. The previous way how PostCSS v7 parsed CSS files to the AST (Abstract Syntax Tree) didn't care about that, but PostCSS v8 changed the parsing logic.

🚫 Invalid:

--marketplaceModalTitleStyles {
  font-size: 30px;
}

✅ Valid:

--marketplaceModalTitleStyles: {
  font-size: 30px;
}

You should only face this issue if you continue to use CSS Property Sets. However, you should consider them as deprecated and migrate away.

There are a couple of warnings with yarn run dev

[0] (node:58738) [DEP_WEBPACK_DEV_SERVER_ON_AFTER_SETUP_MIDDLEWARE] DeprecationWarning: 'onAfterSetupMiddleware' option is deprecated. Please use the 'setupMiddlewares' option.
[0] (Use `node --trace-deprecation ...` to show where the warning was created)
[0] (node:58738) [DEP_WEBPACK_DEV_SERVER_ON_BEFORE_SETUP_MIDDLEWARE] DeprecationWarning: 'onBeforeSetupMiddleware' option is deprecated. Please use the 'setupMiddlewares' option.

This is hopefully fixed in the next release:
facebook/create-react-app#11862

Gnito added 27 commits July 7, 2022 16:52
- h4, clearfix, marketplaceMessageFontStyles
- used h4, h5, buttonFont, and marketplaceTinyFontStyles
- --marketplaceSmallFontStyles was only used here. It's now renamed as smallFontStyles.
- Uses marketplaceModalInMobileBaseStyles
- And h2, h4, h5, and marketplacetineyFontStyles.
- Button styles are also used for bookButton
- a, button styling classes
- Button: composes: button buttonFont buttonText buttonBorders buttonColors from global;
- PrimaryButton: composes: button buttonFont buttonText buttonBorders buttonColorsPrimary from global;
- SecondaryButton: composes: button buttonFont buttonText buttonBordersSecondary buttonColorsSecondary from global;
- marketplaceModalTitleStyles
- marketplaceModalParagraphStyles
- marketplaceModalTitleStyles
- marketplaceModalParagraphStyles
- marketplaceListinAttributeFontStyles
- inlined h2, h4, input, and default font styles
- marketplaceListingAttributeFontStyles
- refactored legend element usage
- inlined h3
- button styles
- composes: button marketplaceSearchFilterLabelFontStyles buttonText buttonBorders buttonColors from global;
- h5 and marketplaceTinyFontStyles
- h4 and button styles
- composes: button marketplaceSearchFilterLabelFontStyles buttonText buttonBordersSecondary buttonColorsSecondary from global;
Gnito added 26 commits July 7, 2022 16:52
- button styles, but inlined custom font
- marketplaceHeroTitleFontStyles, marketplaceTinyFontStyles
- marketplaceModalFormRootStyles, marketplaceModalPasswordMargins, marketplaceModalBottomWrapper,
  marketplaceModalBottomWrapperText, marketplaceModalHelperText, marketplaceModalHelperText
- marketplaceModalFormRootStyles, marketplaceModalPasswordMargins, marketplaceModalBottomWrapper,
marketplaceModalIconStyles, marketplaceModalTitleStyles, marketplaceModalParagraphStyles
- button styles
- marketplaceModalformRootStyles, marketplaceModalBottomWrapper, marketplaceModalBottomWrapperText,
  marketplaceModalHelperLink, marketplaceModalHelperText
- marketplaceModalformRootStyles, marketplaceModalBottomWrapper, marketplaceModalBottomWrapperText,
marketplaceModalHelperLink, marketplaceModalHelperText
- h4, h5, a, marketplaceSearchFilterSublabelFontStyles, marketplaceModalPasswordMargins,
  marketplaceModalHelperText, marketplaceModalHelperLink, marketplaceModalCloseIcon
- h4, h5, marketplaceSearchFilterSublabelFontStyles
- h4,h5, marketplaceDefaultFontStyles
- marketplaceModalFormRootStyles, marketplaceModalPasswordMargins, marketplaceModalBottomWrapper,
  marketplaceModalBottomWrapperText, marketplaceModalHelperText, marketplaceModalHelperLink
- h3, marketplaceModalPasswordMargin, marketplaceModalHelperText, marketplaceModalHleperLink
- inlined h4 styles due to exposed element (p)
@Gnito Gnito force-pushed the update-sharetribe-scripts branch from 25642d7 to 28ec5ab Compare July 7, 2022 13:53
@Gnito Gnito merged commit 267e018 into master Jul 11, 2022
@Gnito Gnito deleted the update-sharetribe-scripts branch July 11, 2022 11:21
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.

None yet

1 participant