✨ improve(@roots/bud-sass) (#2098)

Makes sure @roots/bud-sass does its business before config is called refers: - none ## Type of change **PATCH: backwards compatible change** This PR includes breaking changes to the following core packages: - none This PR includes breaking changes to the follow extensions: - none ## Dependencies ### Adds - none ### Removes - none
roots · Feb 12, 2023 · f31bad2 · f31bad2
1 parent b980f23
commit f31bad2
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 104 deletions.
diff --git a/sources/@roots/bud-sass/src/extension.ts b/sources/@roots/bud-sass/src/extension.ts
@@ -10,13 +10,7 @@ import {
 } from '@roots/bud-framework/extension/decorators'
 
 /**
- * Sass support extension for `@roots/bud-sass`
- *
- * @public
- * @decorator `@label`
- * @decorator `@expose`
- * @decorator `@dependsOn`
- * @decorator `@dependsOnOptional`
+ * Sass configuration
  */
 @label(`@roots/bud-sass`)
 @dependsOn([`@roots/bud-sass/resolve-url`])
@@ -29,50 +23,44 @@ import {
 export class BudSass extends Extension {
  /**
  * `register` callback
- *
- * @public
- * @decorator `@bind`
  */
  @bind
- public override async register(_bud: Bud) {
+ public override async register(bud: Bud) {
  const implementation = await this.import(`sass`)
  this.setOptions({implementation, sourceMap: true})
- }
 
- /**
- * `configAfter` callback
- *
- * @public
- * @decorator `@bind`
- */
- @bind
- public override async configAfter(bud: Bud) {
  bud.build
  .setLoader(`sass-loader`, await this.resolve(`sass-loader`))
  .setItem(`sass`, {
  ident: `sass`,
  loader: `sass-loader`,
- options: this.options,
+ options: () => this.options,
  })
  .setRule(`sass`, {
  test: (app: Bud) => app.hooks.filter(`pattern.sass`),
  include: [app => app.path(`@src`)],
- use: [
- bud.build.items.precss,
- bud.build.items.css,
- bud.build.items.postcss,
- bud.build.items.resolveUrl,
- bud.build.items.sass,
- ].filter(Boolean),
  })
 
  bud.hooks.on(`build.resolve.extensions`, ext =>
  ext.add(`.scss`).add(`.sass`),
  )
+ }
 
+ @bind
+ public override async boot(bud: Bud) {
  if (bud.postcss) {
  bud.postcss.syntax = `postcss-scss`
  }
+
+ bud.build.rules.sass.setUse(() =>
+ [
+ bud.build.items.precss,
+ bud.build.items.css,
+ bud.build.items.postcss,
+ bud.build.items.resolveUrl,
+ bud.build.items.sass,
+ ].filter(Boolean),
+ )
  }
 
  /**
@@ -85,16 +73,17 @@ export class BudSass extends Extension {
  * ```ts
  * bud.sass.registerGlobal(`$primary-color: #ff0000;`)
  * ```
- *
- * @public
- * @decorator `@bind`
  */
  @bind
  public registerGlobal(data: string | Array<string>): this {
- data = Array.isArray(data) ? data : [data]
- return this.setOption(`additionalData`, value =>
- [value ?? null, ...data].filter(Boolean).join(`\n`),
- )
+ return this.set(`additionalData`, (value = ``) => {
+ const code = (Array.isArray(data) ? data : [data])
+ .map(str => str.trim())
+ .filter(Boolean)
+ .join(`\n`)
+
+ return value.concat(code)
+ })
  }
 
  /**
@@ -104,16 +93,27 @@ export class BudSass extends Extension {
  * Used to import a partial globally (such as a `variables.scss` file)
  *
  * @example
+ * With a single module signifier:
  * ```ts
- * bud.sass.importPartial()
+ * bud.sass.importGlobal('styles/variables.scss')
  * ```
  *
- * @public
- * @decorator `@bind`
+ * @example
+ * With an array of module signifiers:
+ * ```ts
+ * bud.sass.importGlobal([
+ * 'styles/variables.scss',
+ * 'styles/mixins.scss',
+ * ])
+ * ```
  */
  @bind
  public importGlobal(data: string | Array<string>): this {
- data = Array.isArray(data) ? data : [data]
- return this.registerGlobal(data.map(item => `@import "${item}";`))
+ const globals = (Array.isArray(data) ? data : [data])
+ .map(str => str.trim())
+ .filter(Boolean)
+ .map(item => `@import "${item}";`)
+
+ return this.registerGlobal(globals)
  }
 }
diff --git a/sources/@roots/bud-sass/src/resolve-url/extension.ts b/sources/@roots/bud-sass/src/resolve-url/extension.ts
@@ -6,18 +6,13 @@ import {label} from '@roots/bud-framework/extension/decorators'
 export class BudResolveUrl extends Extension {
  /**
  * `register` callback
- *
- * @public
  */
  public override async register(bud: Bud) {
  const loader = await this.resolve(`resolve-url-loader`)
 
  bud.build.setLoader(`resolveUrl`, loader).setItem(`resolveUrl`, {
  loader: `resolveUrl`,
- options: ({path, hooks}) => ({
- root: path(`@src`),
- sourceMap: true,
- }),
+ options: ({path}) => ({root: path(`@src`), sourceMap: true}),
  })
  }
 }
diff --git a/sources/@roots/bud-sass/test/extension.test.ts b/sources/@roots/bud-sass/test/extension.test.ts
@@ -4,7 +4,6 @@ import {factory} from '@repo/test-kit/bud'
 import BudPostCSS from '@roots/bud-postcss'
 
 import BudSass from '../src/index.js'
-import { Item } from '@roots/bud-build/item'
 
 describe(`@roots/bud-sass/extension`, () => {
  let bud
@@ -13,6 +12,7 @@ describe(`@roots/bud-sass/extension`, () => {
 
  beforeEach(async () => {
  vi.clearAllMocks()
+
  bud = await factory()
  postcss = new BudPostCSS(bud)
  sass = new BudSass(bud)
@@ -26,93 +26,70 @@ describe(`@roots/bud-sass/extension`, () => {
 
  it(`should call import when sass.register is called`, async () => {
  const importSpy = vi.spyOn(sass, `import`)
-
- try {
- await sass.register(bud)
- } catch (e) {}
+ await sass.register(bud)
  expect(importSpy).toHaveBeenCalled()
  })
 
- it(`should call setOptions when sass.register is called`, async () => {
- const setOptionsSpy = vi.spyOn(sass, `setOptions`)
-
- try {
- await sass.register(bud)
- } catch (e) {}
- expect(setOptionsSpy).toHaveBeenCalled()
- })
-
- it(`should call setOption when sass.registerGlobal is called`, async () => {
- const setOptionSpy = vi.spyOn(sass, `setOption`)
+ it(`should call set when sass.registerGlobal is called`, () => {
+ const setSpy = vi.spyOn(sass, `set`)
 
- try {
- sass.registerGlobal(`$primary-color: #ff0000;`)
- } catch (e) {}
+ sass.registerGlobal(`$primary-color: #ff0000;`)
 
- expect(setOptionSpy).toHaveBeenCalledWith(
+ expect(setSpy).toHaveBeenCalledWith(
  `additionalData`,
  expect.any(Function),
  )
  })
 
- it(`should call setLoader when configAfter is called`, async () => {
+ it(`should call setLoader`, async () => {
  const setLoaderSpy = vi.spyOn(bud.build, `setLoader`)
+ await sass.register(bud)
 
- try {
- await sass.configAfter(bud)
- } catch (e) {}
-
- expect(setLoaderSpy).toHaveBeenCalledWith(`sass-loader`, expect.stringContaining(`sass-loader`))
+ expect(setLoaderSpy).toHaveBeenCalledWith(
+ `sass-loader`,
+ expect.stringContaining(`sass-loader`),
+ )
  })
 
- it(`should call setItem when configAfter is called`, async () => {
+ it(`should call setItem`, async () => {
  const setItemSpy = vi.spyOn(bud.build, `setItem`)
 
- try {
- await sass.configAfter(bud)
- } catch (e) {}
+ await sass.register(bud)
 
  expect(setItemSpy).toHaveBeenCalledWith(
  `sass`,
  expect.objectContaining({
  loader: expect.stringContaining(`sass-loader`),
- options: {
- additionalData: `$primary-color: #ff0000;`,
- implementation: expect.any(Object),
- sourceMap: true,
- },
+ options: expect.any(Function),
  }),
  )
  })
 
- it(`should call setRule when configAfter is called`, async () => {
+ it(`should call setRule`, async () => {
  const setRuleSpy = vi.spyOn(bud.build, `setRule`)
 
- try {
- await sass.configAfter(bud)
- } catch (e) {}
+ await sass.register(bud)
+ await sass.boot(bud)
 
  expect(setRuleSpy).toHaveBeenCalledWith(
  `sass`,
  expect.objectContaining({
  include: expect.arrayContaining([expect.any(Function)]),
  test: expect.any(Function),
- use: expect.arrayContaining([expect.any(Item), expect.any(Item), expect.any(Item), expect.any(Item), expect.any(Item)]),
  }),
  )
  })
 
  it(`should set postcss syntax`, async () => {
+ vi.clearAllMocks()
  postcss.syntax = ``
-
- try {
- await sass.configAfter(bud)
- } catch (e) {}
+ await sass.register(bud)
+ await sass.boot(bud)
 
  expect(sass.app.postcss.syntax).toEqual(`postcss-scss`)
  })
 
- it(`should register global when importGlobal is called`, async () => {
+ it(`should register global when importGlobal is called`, () => {
  const registerGlobalSpy = vi.spyOn(sass, `registerGlobal`)
  sass.importGlobal(`@src/styles/global.scss`)
 
@@ -121,14 +98,14 @@ describe(`@roots/bud-sass/extension`, () => {
  ])
  })
 
- it(`should add global to \`additionalData\``, async () => {
- const setOptionSpy = vi.spyOn(sass, `setOption`)
+ it(`should add global to \`additionalData\``, () => {
+ sass.set(`additionalData`, undefined)
+
+ const setSpy = vi.spyOn(sass, `set`)
 
- sass.setOption(`additionalData`, undefined)
  sass.registerGlobal(`$foo: rgba(0, 0, 0, 1);`)
- await sass.configAfter(bud)
 
- expect(setOptionSpy).toHaveBeenCalledWith(
+ expect(setSpy).toHaveBeenCalledWith(
  `additionalData`,
  expect.any(Function),
  )
@@ -137,15 +114,15 @@ describe(`@roots/bud-sass/extension`, () => {
  )
  })
 
- it(`should import partials from an array`, async () => {
- sass.setOption(`additionalData`, undefined)
- sass.registerGlobal([
+ it(`should import partials from an array`, () => {
+ const code = [
  `$foo: rgba(0, 0, 0, 1);`,
  `$bar: rgba(255, 255, 255, 1);`,
- ])
- await sass.configAfter(bud)
- expect(sass.getOption(`additionalData`)).toBe(
- `$foo: rgba(0, 0, 0, 1);\n$bar: rgba(255, 255, 255, 1);`,
- )
+ ]
+
+ sass.set(`additionalData`, undefined)
+ sass.registerGlobal(code)
+
+ expect(sass.get(`additionalData`).split(`\n`)).toStrictEqual(code)
  })
 })