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.

Sign up for GitHub

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

Jump to bottom

Update sharetribe-scripts to CRA v4 and add vanilla CSS support #1374

Merged
merged 15 commits into from
Nov 17, 2020
Merged

Update sharetribe-scripts to CRA v4 and add vanilla CSS support #1374

merged 15 commits into from
Nov 17, 2020

Conversation

Gnito
Copy link
Contributor

@Gnito Gnito commented Oct 28, 2020 •
edited
Loading

Changes

  • Usage of standard CSS Modules file naming pattern: *.module.css
    Our usage of CSS Modules started before Create React App (CRA) started to use it - and at the time we made a choice to push all the component styles through it. When different frameworks (incl. CRA) started to take CSS Modules into use, the naming pattern of *.module.css started to take hold. When making this update for our CRA fork (sharetribe-scripts), we decided to adopt this naming pattern.
    • => All the style files for components, containers, and forms are renamed.
  • Vanilla CSS can be used with *.css file naming pattern
    • Note: using plain CSS differs a bit from CSS Modules. Those imports don't return exportable object.
  • Live CSS Properties (variables)
    • All the variables are set to :root element scope (aka <html> element) as before,
      but they are now made through a single file: marketplaceDefaults.css.
    • It's possible to change those variables with JS: 
      document.documentElement.style.setProperty("--marketplaceColor", '#55AA55');
  • All the CSS Property Sets (@apply rules) are moved to a single file: propertySets.css
    • propertySets.css file is the only file that needs to be imported using @import in component stylesheets.
    • Adding normal style rules through @import should be avoided since it will be multiplied in dev mode.
      • WebpackDevServer uses <style> elements to include CSS files and @imports would repeat shared code in each <style> tag.
      • However, this doesn't apply to Property Sets, because build injects them inside existing classes - it doesn't leave any extra markup lying around.
    • Only those components that use @apply rule, have the import for propertySets.css file.
      • Previously there was ~170 files that imported marketplace.css, but after these changes, there are ~100 components that need this new file.
  • Global styles are moved under a directory: src/styles/
    • Currently, it includes: marketplaceDefaults.css & propertySets.css
  • Test snapshots are updated since now the class name ident is returned by default. *

*:
Tests needed to be updated because our setup (ComponentName.css) has always been mapped to empty export:

  1. css import goes through custom transformer
  2. cssTransform.js

Now that the CSS Modules is parsed separately, Jest is wired to use "identity-obj-proxy". In practice, that converts className={css.someStyle} to className="someStyle" in Jest snapshots.

It is possible to take the previous setup into use by adding Jest/moduleNameMapper overwrite into package.json.

  "jest": {
    "moduleNameMapper": {
      "^.+\\.module\\.(css|sass|scss)$": "<rootDir>/cssTransform.js"
     }
  },

In addition, there needs to be cssTransform.js in the root directory of the app.

Taking update from upstream

  • If you have made custom components or customized existing ones, you might experience several Git conflicts. This is what we have made to all the components:
    • Rename CSS file: ComponentName.css => ComponentName.module.css
      Otherwise, the file don't go through CSS Modules preprocessor.
    • Rename the import in *.js files:
      import css from 'ComponentName.css' => import css from 'ComponentName.module.css'
    • Move possible CSS Property definitions to marketplaceDefaults.css file.
    • Move possible CSS Property Set definitions to propertySets.css file.
    • If your custom CSS has @import for marketplace.css, remove it.
    • If your custom CSS uses @apply rules (Property Sets) import propertySets.css file
      e.g. @import '../../styles/propertySets.css';
    • Update Jest snapshots (yarn run test)
    • If you have disabled eslint rule eslint-disable jsx-a11y/no-static-element-interactions, remove those from code.

src/styles/marketplaceDefaults.css contains

  • CSS Properties (variables)
  • Custom Media queries
  • Element styles

src/styles/propertySets.css contains

  • CSS Property Sets only
    • You can @import this file to those component-level stylesheets that use them (i.e. there's @apply rule used)

Note:

  • The RegExp to find css imports that are not changed to *.module.css could be created with negative look-behind (tested in Sublime Text): import css from .*(?<!\bmodule\b)\.css
  • You should check all the customizations done to marketplace.css, marketplaceFonts.css and marketplaceIndex.css files before you start the update process.
  • You might want to check all the references to ":root {" element scopes in your CSS files. If you have added custom variables or property sets, you should take a note.
  • You might want to make text comparisons for files where you encounter Git conflicts.

@Gnito Gnito force-pushed the cra4-base branch 3 times, most recently from 7a1761d to 3a0c0f0 Compare November 3, 2020 18:32
@Gnito Gnito force-pushed the cra4-base branch 2 times, most recently from 3920805 to 928ad51 Compare November 17, 2020 00:01
@Gnito Gnito had a problem deploying to sharetribe-starter-app November 17, 2020 12:57 Failure
@Gnito Gnito had a problem deploying to sharetribe-starter-app November 17, 2020 13:47 Failure
@Gnito Gnito temporarily deployed to sharetribe-starter-app November 17, 2020 13:50 Inactive
@Gnito Gnito changed the title WiP: CRA 4 base branch Update sharetribe-scripts with CRA 4 and vanilla CSS support Nov 17, 2020
@Gnito Gnito temporarily deployed to sharetribe-starter-app November 17, 2020 15:40 Inactive
@Gnito Gnito changed the title Update sharetribe-scripts with CRA 4 and vanilla CSS support Update sharetribe-scripts to CRA v4 and add vanilla CSS support Nov 17, 2020
@Gnito Gnito merged commit 1d9d39d into master Nov 17, 2020
This was referenced Nov 17, 2020
Merged
Merged
@Gnito Gnito deleted the cra4-base branch November 18, 2020 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@Gnito