Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix crash on Node.js for Intl.Locale variation #435

Merged
merged 1 commit into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified bun.lockb
Binary file not shown.
7 changes: 6 additions & 1 deletion packages/brisa/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ const IS_SERVE_PROCESS = Boolean(
path.join('brisa', 'out', 'cli', 'serve', 'index.js'),
),
);

const IS_BUILD_PROCESS = Boolean(
process.argv[1]?.endsWith?.(path.join('brisa', 'out', 'cli', 'build.js')),
);

const rootDir = process.cwd();
const staticExportOutputOption = new Set([
'static',
Expand All @@ -27,7 +32,7 @@ const staticExportOutputOption = new Set([
const srcDir = path.resolve(rootDir, 'src');
const buildDir =
process.env.BRISA_BUILD_FOLDER ?? path.resolve(rootDir, 'build');
const WORKSPACE = IS_SERVE_PROCESS ? buildDir : srcDir;
const WORKSPACE = IS_BUILD_PROCESS ? srcDir : buildDir;
Copy link
Collaborator Author

@aralroca aralroca Aug 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only need src building the app, otherwise is always from the build folder. Similar than before, but better, because with a Custom Server is "serve" but was detected as "build", now is better

const PAGE_404 = '/_404';
const PAGE_500 = '/_500';
const integrations = await importFileIfExists(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const i18nCode = 3072;
const brisaSize = 5959; // TODO: Reduce this size :/
const webComponents = 792;
const unsuspenseSize = 217;
const rpcSize = 2466; // TODO: Reduce this size
const rpcSize = 2468; // TODO: Reduce this size
const lazyRPCSize = 4187; // TODO: Reduce this size
// lazyRPC is loaded after user interaction (action, link),
// so it's not included in the initial size
Expand Down
40 changes: 40 additions & 0 deletions packages/brisa/src/utils/render-attributes/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,46 @@ describe('utils', () => {
expect(attributes).toBe(' lang="ru" dir="ltr"');
});

it('should add the "dir" and "lang" when the js-runtime does not support "getTextInfo"', () => {
const request = extendRequestContext({
originalRequest: new Request('https://example.com/ru'),
});

request.i18n = {
locale: 'ru',
locales: ['en', 'ru'],
defaultLocale: 'en',
pages: {},
t: () => '' as any,
overrideMessages: () => {},
};

const originalIntlLocale = Intl.Locale;

Object.assign(Intl, {
Locale: class {
constructor(locale: string) {
return {
textInfo: {
direction: 'ltr',
},
};
}
},
});

const attributes = renderAttributes({
elementProps: {},
request,
type: 'html',
});

Object.assign(Intl, {
Locale: originalIntlLocale,
});
expect(attributes).toBe(' lang="ru" dir="ltr"');
});

it('should modify the existing lang in the "html" tag the ltr direction (when I18N enable)', () => {
const request = extendRequestContext({
originalRequest: new Request('https://example.com/ru'),
Expand Down
14 changes: 8 additions & 6 deletions packages/brisa/src/utils/render-attributes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import type {
IndicatorSignal,
Props,
RequestContext,
Translations,
} from '@/types';
import routeMatchPathname from '@/utils/route-match-pathname';
import { serializeServer } from '@/utils/serialization/server';
Expand Down Expand Up @@ -235,9 +234,12 @@ export default function renderAttributes({
}

if (locale && type === 'html') {
attributes += ` lang="${locale}"`;
const { direction } = (new Intl.Locale(locale) as any).getTextInfo();
attributes += ` dir="${direction}"`;
const localeInfo = new Intl.Locale(locale) as any;
// Note: In some versions of some js-runtimes, "getTextInfo" method
// was implemented as an accessor property called textInfo
const { direction } = localeInfo.textInfo ?? localeInfo.getTextInfo();

attributes += ` lang="${locale}" dir="${direction}"`;
}

if (type === 'head' && basePath) {
Expand All @@ -258,7 +260,7 @@ export function renderHrefAttribute(
let formattedHref = hrefValue.replace(/\/$/, '');

for (const [key, value] of Object.entries(request.route?.params ?? {})) {
formattedHref = formattedHref.replace(`[${key}]`, value);
formattedHref = formattedHref.replace(`[${key}]`, value as string);
}

if (isExternalUrl) {
Expand All @@ -273,7 +275,7 @@ export function renderHrefAttribute(

if (page) {
const [pageName, translations] = page;
const translatedPage = (translations as Translations)?.[locale] ?? pageName;
const translatedPage = (translations as any)?.[locale] ?? pageName;
formattedHref = substituteI18nRouteValues(translatedPage, formattedHref);
}

Expand Down
Loading