Skip to content

Commit 7a4b13e

Browse files
backspacebackspace
authoredMay 2, 2024
host: Add handling for Unicode in file/class names (#1198)
This fixes file creation to support characters that are valid as part of file and identifier names that were previously being stripped. It also leaves the display name alone.
1 parent 3716de0 commit 7a4b13e

26 files changed

+223
-140
lines changed
 

‎packages/host/app/components/card-catalog/item.gts

+1-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ export default class CardCatalogItem extends Component<Signature> {
9393
if (!path) {
9494
return undefined;
9595
}
96-
let realmPath = new RealmPaths(this.cardService.defaultURL.href);
96+
let realmPath = new RealmPaths(this.cardService.defaultURL);
9797

9898
if (/^(\.\.\/)+/.test(path)) {
9999
let localPath = new URL(path, realmPath.url).pathname.replace(/^\//, '');

‎packages/host/app/components/editor/recent-files.gts

+8-5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { fn } from '@ember/helper';
21
import { on } from '@ember/modifier';
32
import { action } from '@ember/object';
43
import { service } from '@ember/service';
@@ -59,11 +58,15 @@ class File extends Component<FileArgs> {
5958
}
6059

6160
get fullUrl() {
62-
return `${this.args.recentFile.realmURL}${this.args.recentFile.filePath}`;
61+
return new URL(
62+
`${this.args.recentFile.realmURL}${this.args.recentFile.filePath}`,
63+
);
6364
}
6465

6566
get isSelected() {
66-
return this.operatorModeStateService.state.codePath?.href === this.fullUrl;
67+
return (
68+
this.operatorModeStateService.state.codePath?.href === this.fullUrl.href
69+
);
6770
}
6871

6972
<template>
@@ -72,9 +75,9 @@ class File extends Component<FileArgs> {
7275
{{! template-lint-disable require-presentational-children }}
7376
<li
7477
class='recent-file'
75-
data-test-recent-file={{this.fullUrl}}
78+
data-test-recent-file={{this.fullUrl.href}}
7679
role='button'
77-
{{on 'click' (fn this.openFile this.fullUrl)}}
80+
{{on 'click' this.openFile}}
7881
>
7982
<RealmInfoProvider @realmURL={{@recentFile.realmURL}}>
8083
<:ready as |realmInfo|>

‎packages/host/app/components/operator-mode/card-url-bar.gts

+1-1
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ export default class CardURLBar extends Component<Signature> {
206206
@service private declare cardService: CardService;
207207

208208
private urlBar: URLBarResource = urlBarResource(this, () => ({
209-
getValue: () => this.codePath,
209+
getValue: () => (this.codePath ? decodeURI(this.codePath) : ''),
210210
setValue: (url: string) => {
211211
this.operatorModeStateService.updateCodePath(new URL(url));
212212
},

‎packages/host/app/components/operator-mode/create-file-modal.gts

+35-15
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,15 @@ import { on } from '@ember/modifier';
44
import { action } from '@ember/object';
55
import type Owner from '@ember/owner';
66
import { service } from '@ember/service';
7-
import { capitalize } from '@ember/string';
87
import { buildWaiter } from '@ember/test-waiters';
98
import Component from '@glimmer/component';
109
import { tracked } from '@glimmer/tracking';
1110

11+
import camelCase from 'camelcase';
1212
import { restartableTask, enqueueTask } from 'ember-concurrency';
1313
import perform from 'ember-concurrency/helpers/perform';
1414
import focusTrap from 'ember-focus-trap/modifiers/focus-trap';
1515
import onKeyMod from 'ember-keyboard/modifiers/on-key';
16-
import camelCase from 'lodash/camelCase';
1716

1817
import {
1918
FieldContainer,
@@ -568,19 +567,14 @@ export default class CreateFileModal extends Component<Signature> {
568567
let {
569568
ref: { name: exportName, module },
570569
} = (this.definitionClass ?? this.selectedCatalogEntry)!; // we just checked above to make sure one of these exists
571-
let className = camelize(this.displayName);
572-
// make sure we don't collide with a javascript built-in object
573-
if (typeof (globalThis as any)[className] !== 'undefined') {
574-
className = `${className}0`;
575-
}
570+
let className = convertToClassName(this.displayName);
571+
576572
let absoluteModule = new URL(module, this.selectedCatalogEntry?.id);
577573
let moduleURL = maybeRelativeURL(
578574
absoluteModule,
579575
url,
580576
this.selectedRealmURL,
581577
);
582-
// sanitize the name since it will be used in javascript code
583-
let safeName = this.displayName.replace(/[^A-Za-z \d-_]/g, '').trim();
584578
let src: string[] = [];
585579

586580
// There is actually only one possible declaration collision: `className` and `parent`,
@@ -590,27 +584,28 @@ export default class CreateFileModal extends Component<Signature> {
590584
import { ${exportName} as ${exportName}Parent } from '${moduleURL}';
591585
import { Component } from 'https://cardstack.com/base/card-api';
592586
export class ${className} extends ${exportName}Parent {
593-
static displayName = "${safeName}";`);
587+
static displayName = "${this.displayName}";`);
594588
} else if (exportName === 'default') {
595-
let parent = camelize(
589+
let parent = camelCase(
596590
module
597591
.split('/')
598592
.pop()!
599593
.replace(/\.[^.]+$/, ''),
594+
{ pascalCase: true },
600595
);
601596
// check for parent/className declaration collision
602597
parent = parent === className ? `${parent}Parent` : parent;
603598
src.push(`
604599
import ${parent} from '${moduleURL}';
605600
import { Component } from 'https://cardstack.com/base/card-api';
606601
export class ${className} extends ${parent} {
607-
static displayName = "${safeName}";`);
602+
static displayName = "${this.displayName}";`);
608603
} else {
609604
src.push(`
610605
import { ${exportName} } from '${moduleURL}';
611606
import { Component } from 'https://cardstack.com/base/card-api';
612607
export class ${className} extends ${exportName} {
613-
static displayName = "${safeName}";`);
608+
static displayName = "${this.displayName}";`);
614609
}
615610
src.push(`\n /*`);
616611
if (this.fileType.id === 'card-definition') {
@@ -730,8 +725,33 @@ export class ${className} extends ${exportName} {
730725
});
731726
}
732727

733-
function camelize(name: string) {
734-
return capitalize(camelCase(name));
728+
export function convertToClassName(input: string) {
729+
// \p{L}: a letter
730+
let invalidLeadingCharactersRemoved = camelCase(
731+
input.replace(/^[^\p{L}_$]+/u, ''),
732+
{ pascalCase: true },
733+
);
734+
735+
if (!invalidLeadingCharactersRemoved) {
736+
let prefixedInput = `Class${input}`;
737+
invalidLeadingCharactersRemoved = camelCase(
738+
prefixedInput.replace(/^[^\p{L}_$]+/u, ''),
739+
{ pascalCase: true },
740+
);
741+
}
742+
743+
let className = invalidLeadingCharactersRemoved.replace(
744+
// \p{N}: a number
745+
/[^\p{L}\p{N}_$]+/gu,
746+
'',
747+
);
748+
749+
// make sure we don't collide with a javascript built-in object
750+
if (typeof (globalThis as any)[className] !== 'undefined') {
751+
className = `${className}0`;
752+
}
753+
754+
return className;
735755
}
736756

737757
const SelectedTypePill: TemplateOnlyComponent<{

‎packages/host/app/components/operator-mode/delete-modal.gts

+1-1
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ export default class DeleteModal extends Component<Signature> {
135135
if (this.item instanceof URL) {
136136
return {
137137
type: 'file',
138-
name: this.item.href.split('/').pop()!,
138+
name: decodeURI(this.item.href).split('/').pop()!,
139139
id: this.item.href,
140140
};
141141
}

‎packages/host/app/components/realm-dropdown.gts

+1-1
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ export default class RealmDropdown extends Component<Signature> {
158158
if (this.args.selectedRealmURL) {
159159
selectedRealm = this.realms.find(
160160
(realm) =>
161-
realm.path === new RealmPaths(this.args.selectedRealmURL as URL).url,
161+
realm.path === new RealmPaths(this.args.selectedRealmURL!).url,
162162
);
163163
}
164164
if (selectedRealm) {

‎packages/host/app/resources/file.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -182,14 +182,15 @@ class _FileResource extends Resource<Args> {
182182
await realmSession.loaded;
183183

184184
let self = this;
185+
let rawName = response.url.split('/').pop();
185186

186187
this.updateState({
187188
state: 'ready',
188189
lastModified,
189190
realmURL,
190191
content,
191192
realmSession,
192-
name: response.url.split('/').pop()!,
193+
name: rawName ? decodeURIComponent(rawName) : rawName!,
193194
size,
194195
url: response.url,
195196
write(content: string, flushLoader?: true) {

‎packages/host/app/services/card-service.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,7 @@ export default class CardService extends Service {
439439

440440
getRealmURLFor(url: URL) {
441441
for (let realmURL of this.realmURLs) {
442-
let path = new RealmPaths(realmURL);
442+
let path = new RealmPaths(new URL(realmURL));
443443
if (path.inRealm(url)) {
444444
return new URL(realmURL);
445445
}

‎packages/host/app/services/operator-mode-state-service.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ export default class OperatorModeStateService extends Service {
145145

146146
let cardRealmUrl = await this.cardService.getRealmURL(card);
147147
let realmPaths = new RealmPaths(cardRealmUrl);
148-
let cardPath = realmPaths.local(`${card.id}.json`);
148+
let cardPath = realmPaths.local(new URL(`${card.id}.json`));
149149
this.recentFilesService.removeRecentFile(cardPath);
150150
await this.cardService.deleteCard(card);
151151
}

‎packages/host/app/services/realm-info-service.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ export default class RealmInfoService extends Service {
124124

125125
fetchAllKnownRealmInfos = restartableTask(async () => {
126126
let paths = this.cardService.realmURLs.map(
127-
(path) => new RealmPaths(path).url,
127+
(path) => new RealmPaths(new URL(path)).url,
128128
);
129129
let token = waiter.beginAsync();
130130
try {

‎packages/host/app/services/recent-files-service.ts

+5-3
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ export default class RecentFilesService extends Service {
4646
this.persistRecentFiles();
4747
}
4848

49-
addRecentFileUrl(url: string) {
50-
if (!url) {
49+
addRecentFileUrl(urlString: string) {
50+
if (!urlString) {
5151
return;
5252
}
5353
// TODO this wont work when visiting files that come from multiple realms in
@@ -56,7 +56,9 @@ export default class RecentFilesService extends Service {
5656

5757
if (realmURL) {
5858
let realmPaths = new RealmPaths(new URL(realmURL));
59-
if (realmPaths.inRealm(new URL(url))) {
59+
let url = new URL(urlString);
60+
61+
if (realmPaths.inRealm(url)) {
6062
this.addRecentFile(realmPaths.local(url));
6163
}
6264
}

‎packages/host/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@
7777
"broccoli-plugin": "^4.0.7",
7878
"broccoli-source": "^3.0.1",
7979
"buffer": "^6.0.3",
80+
"camelcase": "^6.3.0",
8081
"concurrently": "^8.2.2",
8182
"crypto-browserify": "^3.12.0",
8283
"date-fns": "^2.28.0",

‎packages/host/tests/acceptance/code-submode/create-file-test.gts

+10-54
Original file line numberDiff line numberDiff line change
@@ -556,12 +556,12 @@ module('Acceptance | code submode | create-file tests', function (hooks) {
556556
});
557557

558558
test<TestContextWithSave>('can create a new card definition in different realm than inherited definition', async function (assert) {
559-
assert.expect(9);
559+
assert.expect(11);
560560
let expectedSrc = `
561561
import { CardDef } from 'https://cardstack.com/base/card-api';
562562
import { Component } from 'https://cardstack.com/base/card-api';
563-
export class TestCard extends CardDef {
564-
static displayName = "Test Card";
563+
export class TrèsTestCard extends CardDef {
564+
static displayName = "Très test card 😀";
565565
566566
/*
567567
static isolated = class Isolated extends Component<typeof this> {
@@ -587,14 +587,14 @@ export class TestCard extends CardDef {
587587
assert
588588
.dom('[data-test-create-definition]')
589589
.isDisabled('create button is disabled');
590-
await fillIn('[data-test-display-name-field]', 'Test Card');
590+
await fillIn('[data-test-display-name-field]', 'Très test card 😀');
591591
assert
592592
.dom(`[data-test-inherits-from-field] [data-test-boxel-field-label]`)
593593
.hasText('Inherits From');
594594
assert
595595
.dom('[data-test-create-definition]')
596596
.isDisabled('create button is disabled');
597-
await fillIn('[data-test-file-name-field]', 'test-card');
597+
await fillIn('[data-test-file-name-field]', 'très-test-card');
598598
assert
599599
.dom('[data-test-create-definition]')
600600
.isEnabled('create button is enabled');
@@ -614,7 +614,11 @@ export class TestCard extends CardDef {
614614
'monaco displays the new definition',
615615
);
616616

617-
await waitFor('[data-test-card-schema="Test Card"]');
617+
await waitFor('[data-test-card-schema="Très test card 😀"]');
618+
assert.dom('[data-test-current-module-name]').hasText('très-test-card.gts');
619+
assert
620+
.dom('[data-test-card-url-bar-input]')
621+
.hasValue(`${testRealmURL}très-test-card.gts`);
618622
assert
619623
.dom('[data-test-card-schema]')
620624
.exists({ count: 3 }, 'the card hierarchy is displayed in schema editor');
@@ -957,54 +961,6 @@ export class Map0 extends Pet {
957961
await deferred.promise;
958962
});
959963

960-
test<TestContextWithSave>('can sanitize display name when creating a new definition', async function (assert) {
961-
assert.expect(1);
962-
await visitOperatorMode(this.owner);
963-
await openNewFileModal('Card Definition');
964-
965-
await fillIn('[data-test-display-name-field]', 'Test Card; { }');
966-
await fillIn('[data-test-file-name-field]', 'test-card');
967-
let deferred = new Deferred<void>();
968-
this.onSave((_, content) => {
969-
if (typeof content !== 'string') {
970-
throw new Error(`expected string save data`);
971-
}
972-
assert.strictEqual(
973-
content,
974-
`
975-
import { CardDef } from 'https://cardstack.com/base/card-api';
976-
import { Component } from 'https://cardstack.com/base/card-api';
977-
export class TestCard extends CardDef {
978-
static displayName = "Test Card";
979-
980-
/*
981-
static isolated = class Isolated extends Component<typeof this> {
982-
<template></template>
983-
}
984-
985-
static embedded = class Embedded extends Component<typeof this> {
986-
<template></template>
987-
}
988-
989-
static atom = class Atom extends Component<typeof this> {
990-
<template></template>
991-
}
992-
993-
static edit = class Edit extends Component<typeof this> {
994-
<template></template>
995-
}
996-
*/
997-
}`.trim(),
998-
'the source is correct',
999-
);
1000-
deferred.fulfill();
1001-
});
1002-
1003-
await click('[data-test-create-definition]');
1004-
await waitFor('[data-test-create-file-modal]', { count: 0 });
1005-
await deferred.promise;
1006-
});
1007-
1008964
test<TestContextWithSave>('can specify new directory as part of filename when creating a new definition', async function (assert) {
1009965
assert.expect(2);
1010966
let expectedSrc = `

‎packages/host/tests/acceptance/code-submode/inspector-test.ts

+5-4
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,7 @@ module('Acceptance | code submode | inspector tests', function (hooks) {
467467
},
468468
},
469469
},
470+
'léame.md': 'hola mundo',
470471
'readme.md': 'hello world',
471472
'not-json.json': 'I am not JSON.',
472473
'Person/1.json': {
@@ -1226,21 +1227,21 @@ module('Acceptance | code submode | inspector tests', function (hooks) {
12261227
await visitOperatorMode({
12271228
stacks: [[]],
12281229
submode: 'code',
1229-
codePath: `${testRealmURL}readme.md`,
1230+
codePath: `${testRealmURL}léame.md`,
12301231
});
12311232
await waitForCodeEditor();
12321233
assert.dom('[data-test-delete-modal-container]').doesNotExist();
12331234

12341235
await waitFor(`[data-test-action-button="Delete"]`);
12351236
await click('[data-test-action-button="Delete"]');
1236-
await waitFor(`[data-test-delete-modal="${testRealmURL}readme.md"]`);
1237+
await waitFor(`[data-test-delete-modal="${testRealmURL}l%C3%A9ame.md"]`);
12371238
assert
12381239
.dom('[data-test-delete-msg]')
1239-
.includesText('Delete the file readme.md?');
1240+
.includesText('Delete the file léame.md?');
12401241
await click('[data-test-confirm-delete-button]');
12411242
await waitFor('[data-test-empty-code-mode]');
12421243

1243-
let notFound = await adapter.openFile('readme.md');
1244+
let notFound = await adapter.openFile('léame.md');
12441245
assert.strictEqual(notFound, undefined, 'file ref does not exist');
12451246
assert.dom('[data-test-delete-modal-container]').doesNotExist();
12461247
});

0 commit comments

Comments
 (0)