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

Extraction breaks for whole file when using shorthand JSX fragments #86

Closed
beheh opened this issue Jul 5, 2018 · 9 comments
Closed

Comments

@beheh
Copy link
Contributor

beheh commented Jul 5, 2018

When attempting to parse a file on 2.6.2 that contains a shorthand JSX fragment (<>like this</>), the scanner fails to parse the whole file and doesn't include any string from it.

While we can replace the fragments in our code base with (<React.Fragment>like this</React.Fragment>), we'd like to be able to use them. The biggest problem is that the existance of one shorthand fragment breaks a whole file, not just the Trans components using it/inside it.

From a cursory look it seems like acorn-jsx-walk hasn't been updated since 2016, so it may be using some old babel or custom code under the hood that is unable to cope with shorthand fragments. When running i18next-scanner in debug mode, I get the following message:

{ SyntaxError: Unexpected token (3:38)
    at Parser.pp.raise (/opt/source/node_modules/acorn-jsx-walk/node_modules/acorn/dist/acorn.js:943:13)
    at Parser.pp.unexpected (/opt/source/node_modules/acorn-jsx-walk/node_modules/acorn/dist/acorn.js:1503:8)
    at Parser.pp.expect (/opt/source/node_modules/acorn-jsx-walk/node_modules/acorn/dist/acorn.js:1497:26)
    at Parser.pp.parseBindingList (/opt/source/node_modules/acorn-jsx-walk/node_modules/acorn/dist/acorn.js:1149:40)
    at Parser.pp.parseFunctionParams (/opt/source/node_modules/acorn-jsx-walk/node_modules/acorn/dist/acorn.js:2071:22)
    at Parser.pp.parseFunction (/opt/source/node_modules/acorn-jsx-walk/node_modules/acorn/dist/acorn.js:2064:8)
    at Parser.pp.parseFunctionStatement (/opt/source/node_modules/acorn-jsx-walk/node_modules/acorn/dist/acorn.js:1830:15)
    at Parser.pp.parseStatement (/opt/source/node_modules/acorn-jsx-walk/node_modules/acorn/dist/acorn.js:1709:19)
    at Parser.pp.parseExport (/opt/source/node_modules/acorn-jsx-walk/node_modules/acorn/dist/acorn.js:2169:29)
    at Parser.pp.parseStatement (/opt/source/node_modules/acorn-jsx-walk/node_modules/acorn/dist/acorn.js:1741:85) pos: 91, loc: Position { line: 3, column: 38 }, raisedAt: 92 }

Writing a test should be trivial.

Version

  • i18next: 11.3.3
  • i18next-scanner: 2.6.2

Configuration

const fs = require("fs");

module.exports = {
	options: {
		// use strings as keys
		nsSeparator: false,
		keySeparator: false,
		// settings
		defaultNs: "frontend",
		lngs: ["en"],
		resource: {
			loadPath: "{{lng}}/{{ns}}.json",
			savePath: "{{lng}}/{{ns}}.json",
		},
		func: false,
		trans: false,
		defaultValue: (language, namespace, key) => key,
	},
	transform(file, enc, done) {
		const parser = this.parser;
		const content = fs.readFileSync(file.path, enc);

		parser.parseFuncFromString(
			content,
			{
				list: ["t"],
			},
			(key, options) => {
				if (["some", "blacklisted", "keys"].indexOf(key) !== -1) {
					return;
				}
				parser.set(key, options);
			},
		);

		parser.parseTransFromString(
			content,
			{
				component: ["Trans"],
				i18nKey: false,
			},
			(key, options) => {
				parser.set(options.defaultValue, options);
			},
		);

		done();
	},
};
@beheh
Copy link
Contributor Author

beheh commented Jul 11, 2018

I have a PR ready which fixes this by replaces the unmaintained acorn-jsx-walk with the much more modern acorn-jsx. It still needs this change to be merged into and released by acorn-jsx, but as soon as it's out I'll submit my PR here.

@spencermefford
Copy link

I am also having issues with the scanner and parsing some ES6 features. Looking forward to this!

@beheh
Copy link
Contributor Author

beheh commented Jul 18, 2018

Yeah, still waiting for that PR to get merged upstream unfortunately :-/

@MiroslavPetrik
Copy link

MiroslavPetrik commented Jul 23, 2018

Up.

Buggy, it even can not extract translations from props e.g. <Component title={<Trans>wont be extracted</Trans>} />

@beheh
Copy link
Contributor Author

beheh commented Jul 23, 2018

@MiroslavPetrik Unless you're using shorthand JSX fragments you should probably open a separate issue.

@cheton
Copy link
Member

cheton commented Jul 23, 2018

@MiroslavPetrik I opened a new issue #90 for this case.


Fixed in v2.6.4 with test cases below:

<Component render={(<Trans>translation from props</Trans>)} />
<Component render={(<Component render={(<Trans>translation from nested props</Trans>)} />)} />

@ctavan
Copy link
Contributor

ctavan commented Nov 28, 2018

@beheh can you check if #112 (npm install @contentpass/i18next-scanner) fixes the issue for you?

@ctavan
Copy link
Contributor

ctavan commented Nov 29, 2018

@beheh since my patch has been released, can you check again with [email protected]?

@beheh
Copy link
Contributor Author

beheh commented Jan 9, 2019

@ctavan Sorry for the delay and thanks for your work, this is indeed fixed now!

@beheh beheh closed this as completed Jan 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants