Skip to content

Commit

Permalink
Added in a check on script tags in theme app extensions to make sure …
Browse files Browse the repository at this point in the history
…importmap is not being used
  • Loading branch information
AribaRajput committed Jan 2, 2025
1 parent b9237f9 commit 99c2512
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 293 deletions.
5 changes: 0 additions & 5 deletions .changeset/chilled-bugs-juggle.md

This file was deleted.

5 changes: 5 additions & 0 deletions .changeset/yellow-insects-draw.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/theme-check-common': minor
---

This theme check will check for and suggest removing script tags with type importmap on theme app extensions
4 changes: 2 additions & 2 deletions packages/theme-check-common/src/checks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import { ValidSchemaName } from './valid-schema-name';
import { ValidStaticBlockType } from './valid-static-block-type';
import { VariableName } from './variable-name';
import { MissingSchema } from './missing-schema';
import { ValidBlockPresetSettings } from './valid-block-preset-settings';
import { NoImportMap } from './no-import-map';

export const allChecks: (LiquidCheckDefinition | JSONCheckDefinition)[] = [
AppBlockValidTags,
Expand Down Expand Up @@ -93,7 +93,7 @@ export const allChecks: (LiquidCheckDefinition | JSONCheckDefinition)[] = [
ValidStaticBlockType,
VariableName,
ValidSchemaName,
ValidBlockPresetSettings,
NoImportMap,
];

/**
Expand Down
31 changes: 31 additions & 0 deletions packages/theme-check-common/src/checks/no-import-map/index.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { describe, expect, it } from 'vitest';
import { NoImportMap } from '.';
import { check as reportOffenses } from '../../test';

describe('Module: NoImportMap', () => {
it('should report offense when using <script type="importmap">', async () => {
const file = `<script type="importmap">
{
"imports": {
"a.js": "https://foo.bar/baz.js"
}
}
</script>`;
const startIndex = file.indexOf('<script');
const endIndex = file.indexOf('</script>') + '</script>'.length;

const offenses = await reportOffenses({ 'code.liquid': file }, [NoImportMap]);

expect(offenses).to.have.length(1);
const { message, start, end } = offenses[0];

expect(message).toEqual(
'Until browsers permit multiple importmap entries, only themes can have an importmap',
);
expect(start.index).toEqual(startIndex);
expect(end.index).toEqual(endIndex);

expect(offenses[0].suggest).to.have.length(1);
expect(offenses[0]!.suggest![0].message).to.equal("Remove the 'importmap' script tag");
});
});
54 changes: 54 additions & 0 deletions packages/theme-check-common/src/checks/no-import-map/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { ConfigTarget, LiquidCheckDefinition, Severity, SourceCodeType } from '../../types';

import { hasAttributeValueOf, isAttr, isValuedHtmlAttribute } from '../utils';

export const NoImportMap: LiquidCheckDefinition = {
meta: {
code: 'NoImportMap',
name: 'Import map in theme app extensions',
docs: {
description:
'Report offenses associated with import maps on script tags in theme app extensions',
url: 'https://shopify.dev/docs/storefronts/themes/tools/theme-check/checks/no-import-map',
},
type: SourceCodeType.LiquidHtml,
severity: Severity.ERROR,
schema: {},
targets: [ConfigTarget.ThemeAppExtension],
},

create(context) {
return {
async HtmlRawNode(node) {
if (node.name !== 'script') {
return;
}

const typeImportMap = node.attributes
.filter(isValuedHtmlAttribute)
.some((attr) => isAttr(attr, 'type') && hasAttributeValueOf(attr, 'importmap'));

const typeModule = node.attributes
.filter(isValuedHtmlAttribute)
.some((attr) => isAttr(attr, 'type') && hasAttributeValueOf(attr, 'importmap'));

if (!typeImportMap || !typeModule) {
return;
}

context.report({
message:
'Until browsers permit multiple importmap entries, only themes can have an importmap',
startIndex: node.position.start,
endIndex: node.position.end,
suggest: [
{
message: `Remove the 'importmap' script tag`,
fix: (corrector) => corrector.remove(node.position.start, node.position.end),
},
],
});
},
};
},
};

This file was deleted.

This file was deleted.

Loading

0 comments on commit 99c2512

Please sign in to comment.