Summary

Redo the build process to address path and reference generation issues.

Review Request #13942 — Created June 5, 2024 and submitted June 5, 2024, 4:57 p.m.

Information

Owner

chipx86

Repository

Ink

Branch

master

Bugs

Depends On

Reviewers

Groups

beanbag-misc

People

Description

There were a couple things going wrong here:

We had an @images-path variable in LessCSS that was used to link to
the Ink images, but this could conflict with consumers and was
referencing an ink directory that wasn't going to consistently
exist in the build process between local builds and remote
consumption.
We were generating bad sourcemaps due to what appears to be issues in
cleancss, depending on where it's run.
We had some build configuration missing that impacted lookup of
Spina from the global object.

This change redoes a lot of our build process to address this. We're now
using Rollup for all builds, rather than scripts within package.json.

We build the CSS at the same time we build our TypeScript types file. We
have to key off rules based on some JavaScript file, so we choose that,
since it only produces one output, ensuring we only do this work once.

The order of operations is more closely managed, as well as the
directory in which we're running cleancss, helping to resolve
previously-hidden warnings about sourcemaps.

For the image references, we now have an @ink-path variable that
points to @beanbag/ink/lib, which is what would be needed by consumers
of the LEss files. Paths are built off of this, rather than the old
@images-path.

For builds and for Storybook runtimes, this variable gets overridden to
point to the right place, ensuring we're accessing images from the right
place.

We're also now ensuring files are built before publishing, avoiding any
bad uploaded builds.

Through this, we should now have proper builds going forward, with paths
referenced correctly.

Testing Done

Built the package and compared to a released build. Verified that the only

changes made were fixes to the images and Spina references.

Commits

Summary	ID
Redo the build process to address path and reference generation issues. The resulting Ink builds had bad image paths that were breaking any post-processing, consumption or the build, and references to the images. We have a couple images that we normally try to inline into the CSS, but due to path issues in the build setup, we'd end up instead generating CSS that referenced relative paths. There were a couple things going wrong here: 1. We had an `@images-path` variable in LessCSS that was used to link to the Ink images, but this could conflict with consumers and was referencing an `ink` directory that wasn't going to consistently exist in the build process between local builds and remote consumption. 2. We were generating bad sourcemaps due to what appears to be issues in `cleancss`, depending on where it's run. 3. We had some build configuration missing that impacted lookup of `Spina` from the global object. This change redoes a lot of our build process to address this. We're now using Rollup for all builds, rather than scripts within `package.json`. We build the CSS at the same time we build our TypeScript types file. We have to key off rules based on some JavaScript file, so we choose that, since it only produces one output, ensuring we only do this work once. The order of operations is more closely managed, as well as the directory in which we're running `cleancss`, helping to resolve previously-hidden warnings about sourcemaps. For the image references, we now have an `@ink-path` variable that points to `@beanbag/ink/lib`, which is what would be needed by consumers of the LEss files. Paths are built off of this, rather than the old `@images-path`. For builds and for Storybook runtimes, this variable gets overridden to point to the right place, ensuring we're accessing images from the right place. Through this, we should now have proper builds going forward, with paths referenced correctly.	e69a87d18012077b1b840103845e1b3c0b1683f7

Summary

Redo the build process to address path and reference generation issues.

The resulting Ink builds had bad image paths that were breaking any post-processing, consumption or the build, and references to the images. We have a couple images that we normally try to inline into the CSS, but due to path issues in the build setup, we'd end up instead generating CSS that referenced relative paths. There were a couple things going wrong here: 1. We had an `@images-path` variable in LessCSS that was used to link to the Ink images, but this could conflict with consumers and was referencing an `ink` directory that wasn't going to consistently exist in the build process between local builds and remote consumption. 2. We were generating bad sourcemaps due to what appears to be issues in `cleancss`, depending on where it's run. 3. We had some build configuration missing that impacted lookup of `Spina` from the global object. This change redoes a lot of our build process to address this. We're now using Rollup for all builds, rather than scripts within `package.json`. We build the CSS at the same time we build our TypeScript types file. We have to key off rules based on some JavaScript file, so we choose that, since it only produces one output, ensuring we only do this work once. The order of operations is more closely managed, as well as the directory in which we're running `cleancss`, helping to resolve previously-hidden warnings about sourcemaps. For the image references, we now have an `@ink-path` variable that points to `@beanbag/ink/lib`, which is what would be needed by consumers of the LEss files. Paths are built off of this, rather than the old `@images-path`. For builds and for Storybook runtimes, this variable gets overridden to point to the right place, ensuring we're accessing images from the right place. Through this, we should now have proper builds going forward, with paths referenced correctly.

e69a87d18012077b1b840103845e1b3c0b1683f7

Issues

Description	From	Last Updated
'lessIncludePath' is defined but never used. Column: 7 Error code: W098	reviewbot	June 5, 2024, 1:07 p.m.
'buildPath' is defined but never used. Column: 7 Error code: W098	reviewbot	June 5, 2024, 1:07 p.m.
Looks like the changes from /r/13939/ were included in this change. And this line in particular has a discrepancy between …	maubin	June 5, 2024, 3:04 p.m.

flake8 passed.

JSHint failed.

JSHint

rollup.config.js (Diff revision 1)

The issue has been resolved. Show all issues

'lessIncludePath' is defined but never used.

Column: 7
Error code: W098

rollup.config.js (Diff revision 1)
The issue has been resolved. Show all issues
```
'buildPath' is defined but never used.

Column: 7
Error code: W098
```

Commits:

	Summary	ID
	Redo the build process to address path and reference generation issues. The resulting Ink builds had bad image paths that were breaking any post-processing, consumption or the build, and references to the images. We have a couple images that we normally try to inline into the CSS, but due to path issues in the build setup, we'd end up instead generating CSS that referenced relative paths. There were a couple things going wrong here: 1. We had an `@images-path` variable in LessCSS that was used to link to the Ink images, but this could conflict with consumers and was referencing an `ink` directory that wasn't going to consistently exist in the build process between local builds and remote consumption. 2. We were generating bad sourcemaps due to what appears to be issues in `cleancss`, depending on where it's run. 3. We had some build configuration missing that impacted lookup of `Spina` from the global object. This change redoes a lot of our build process to address this. We're now using Rollup for all builds, rather than scripts within `package.json`. We build the CSS at the same time we build our TypeScript types file. We have to key off rules based on some JavaScript file, so we choose that, since it only produces one output, ensuring we only do this work once. The order of operations is more closely managed, as well as the directory in which we're running `cleancss`, helping to resolve previously-hidden warnings about sourcemaps. For the image references, we now have an `@ink-path` variable that points to `@beanbag/ink/lib`, which is what would be needed by consumers of the LEss files. Paths are built off of this, rather than the old `@images-path`. For builds and for Storybook runtimes, this variable gets overridden to point to the right place, ensuring we're accessing images from the right place. Through this, we should now have proper builds going forward, with paths referenced correctly.	4bb5b633a06e9d6b1d4de59f5800c95f2b18cdb1
	Redo the build process to address path and reference generation issues. The resulting Ink builds had bad image paths that were breaking any post-processing, consumption or the build, and references to the images. We have a couple images that we normally try to inline into the CSS, but due to path issues in the build setup, we'd end up instead generating CSS that referenced relative paths. There were a couple things going wrong here: 1. We had an `@images-path` variable in LessCSS that was used to link to the Ink images, but this could conflict with consumers and was referencing an `ink` directory that wasn't going to consistently exist in the build process between local builds and remote consumption. 2. We were generating bad sourcemaps due to what appears to be issues in `cleancss`, depending on where it's run. 3. We had some build configuration missing that impacted lookup of `Spina` from the global object. This change redoes a lot of our build process to address this. We're now using Rollup for all builds, rather than scripts within `package.json`. We build the CSS at the same time we build our TypeScript types file. We have to key off rules based on some JavaScript file, so we choose that, since it only produces one output, ensuring we only do this work once. The order of operations is more closely managed, as well as the directory in which we're running `cleancss`, helping to resolve previously-hidden warnings about sourcemaps. For the image references, we now have an `@ink-path` variable that points to `@beanbag/ink/lib`, which is what would be needed by consumers of the LEss files. Paths are built off of this, rather than the old `@images-path`. For builds and for Storybook runtimes, this variable gets overridden to point to the right place, ensuring we're accessing images from the right place. Through this, we should now have proper builds going forward, with paths referenced correctly.	e69a87d18012077b1b840103845e1b3c0b1683f7

Diff:

Revision 2 (+168 -44)

Show changes

	package.json
	package-lock.json
	rollup.config.js
	vite.config.js
	src/ink/less/index.less
	src/ink/less/themes/default/resources.less
	src/ink/less/themes/default/components/spinner.theme.less

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

src/ink/less/index.less (Diff revision 2)

The issue has been dropped. Show all issues

Looks like the changes from /r/13939/ were included in this change. And this line in particular has a discrepancy between the line in /r/13939/.

chipx86 June 5, 2024, 3:04 p.m.

Oh, that change actually should have been full-on replaced by this change. More was needed than just replacing those paths. I guess I didn't update that one.

Status:: Completed
Change Summary:: Pushed to master (70f9238)