openedx / openedx/paragon

Paragon's 'dist' build is not full compiled to JavaScript, won't run without a bundler

Open
#3,322 3 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
JavaScript
Stars
140
Forks
100
Avg merge
1h 3m
Merged PRs (30d)
30

Description

I tried to use Paragon in an XBlock that's using Rollup to transpile its .tsx source code + dependencies into a single .js file. However, I ran into these problems:

 [!] RollupError: node_modules/@openedx/paragon/scss/core/_exports.module.scss (6:17): Expected ';', '}' or <eof>
 (Note that you need plugins to import files that are not JavaScript)
 [!] RollupError: node_modules/@openedx/paragon/dist/Avatar/default-avatar.svg (1:0): Expression expected
 (Note that you need plugins to import files that are not JavaScript)
 [!] RollupError: node_modules/@openedx/paragon/dist/Card/fallback-default.png (1:0): Unexpected character '�'
 (Note that you need plugins to import files that are not JavaScript)

It turns out that even in our dist directory, the JS code is trying to import things that are not JavaScript. For example:

In paragon/dist/utils/breakpoints.js:

import breakpointSizes from '../../scss/core/_exports.module.scss';

In paragon/dist/Avatar/index.js:

import defaultAvatar from './default-avatar.svg';

In paragon/dist/Card/CardImageCap.js:

import cardSrcFallbackImg from './fallback-default.png';

Discussion

Breakpoints:

It seems like the breakpoint sizes themselves (sm, md, lg, xl, xxl) are hard-coded, but the values are customizable, and the intent of the code in breakpoints.js is to read those pixel width values from SCSS variables so they can be customized. If we want to allow people to customize the breakpoints using SCSS variables, and have those available to JavaScript, we should be using CSS variables instead of SCSS variables. Then the JS can just check the CSS variables, and doesn't have to import SCSS at all. This is not only cleaner, but should make the build faster.

However, perhaps the design tokens work already makes this problem irrelevant?

Avatar SVG:

This just seems like a build configuration setting. Our build process should be able to inline the SVG and bundle it into the final .js file. We already do that for the icon SVGs, for example. Not sure why that's not happening.

Card Image PNG:

We shouldn't be including PNG images in the project at all IMHO. The image in question is just a grey rectangle so I think it should be easily replaced by an SVG or even no image at all. If we must include a PNG, it should be a tiny, highly-optimized data: URL.

Other dist optimizations

Please see my vaguely related PR https://github.com/openedx/paragon/pull/3284 which is waiting for review + approval :)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the Rollup errors, then inspect dist/utils/breakpoints.js, dist/Avatar/index.js, and dist/Card/CardImageCap.js alongside the build handling for icon SVGs. Determine how the dist build should package the SCSS, SVG, and PNG dependencies; done means the affected dist JavaScript can be consumed by Rollup without requiring non-JavaScript imports.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, react, rollup, scss
Domain
build-system, frontend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.