-
Notifications
You must be signed in to change notification settings - Fork 652
PRO-6295: jsx as an optional alternative to nunjucks #5391
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
Merged
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
8c0599b
jsx as an optional alternative to nunjucks
boutell 680931a
eslint, all tests pass
boutell 727d258
log a useful stack trace on attachment errors! Holy shit!
boutell ae7f992
Merge branch 'main' into jsx
boutell 583c8cb
fix lint
boutell b10be5b
clarify behavior
boutell 5b52205
more tests
boutell b9db553
This is just a unit test, but it can't hurt to be thorough & satisfy …
boutell 4f19c90
true access to the apos object in jsx, per the spec
boutell 0a623a0
more test coverage, no code changes
boutell 8bacb27
fix watchers
boutell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| // Quick syntax check for every .jsx template under public-demo. Loads | ||
| // each via the same JSX loader used in production and reports compile | ||
| // errors with proper file/line info. | ||
| import path from 'node:path'; | ||
| import { fileURLToPath } from 'node:url'; | ||
| import { createRequire } from 'node:module'; | ||
| import fs from 'node:fs'; | ||
|
|
||
| const here = path.dirname(fileURLToPath(import.meta.url)); | ||
| const apostropheRoot = path.resolve(here, '..'); | ||
| const require = createRequire(path.join(apostropheRoot, 'packages/apostrophe/index.js')); | ||
|
|
||
| const { install } = require(path.join(apostropheRoot, 'packages/apostrophe/modules/@apostrophecms/template/lib/jsxLoader.js')); | ||
| install(); | ||
|
|
||
| const demoRoot = '/srv/workspace/apostrophecms/public-demo'; | ||
|
|
||
| function* walk(dir) { | ||
| for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { | ||
| if (entry.name === 'node_modules' || entry.name === 'data') continue; | ||
| const full = path.join(dir, entry.name); | ||
| if (entry.isDirectory()) { | ||
| yield* walk(full); | ||
| } else if (entry.name.endsWith('.jsx')) { | ||
| yield full; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| let failed = 0; | ||
| for (const file of walk(demoRoot)) { | ||
| try { | ||
| require(file); | ||
| console.log('OK ', file); | ||
| } catch (err) { | ||
| failed += 1; | ||
| console.error('FAIL', file); | ||
| console.error(' ', err.message); | ||
| if (err.stack) { | ||
| console.error(err.stack.split('\n').slice(1, 5).join('\n')); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (failed > 0) { | ||
| console.error(`\n${failed} file(s) failed to compile/load.`); | ||
| process.exit(1); | ||
| } | ||
| console.log(`\nAll JSX templates loaded successfully.`); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,6 +76,32 @@ module.exports = { | |
| self.insertions = {}; | ||
| self.runtimeNodes = {}; | ||
|
|
||
| // Install the .jsx require hook and teach the JSX runtime about | ||
| // Nunjucks' SafeString class so its instances pass through unescaped. | ||
| self.initJsx(); | ||
|
|
||
| // Wire up the view-folder watcher with the two default invalidation | ||
| // handlers — Nunjucks loader caches and compiled .jsx modules. Both | ||
| // engines share a single set of chokidar watchers so we don't pay | ||
| // twice for watching the same directories. | ||
| const jsxLoader = require('./lib/jsxLoader.js'); | ||
| self.onViewChange(function clearNunjucksLoaderCaches() { | ||
| // Setting `cache = {}` mirrors the historical in-loader behavior | ||
| // and is exactly what Nunjucks itself reads when looking up a | ||
| // previously-loaded template. | ||
| for (const loader of Object.values(self.loaders || {})) { | ||
| loader.cache = {}; | ||
| } | ||
| }); | ||
| self.onViewChange(function invalidateJsxModules(filePath) { | ||
| if (filePath && filePath.endsWith('.jsx')) { | ||
| jsxLoader.invalidate(path.resolve(filePath)); | ||
| } else { | ||
| // Anything else (e.g. a Nunjucks file) might be a template imported | ||
| // by a `.jsx` file via require()/import — be safe and drop them all. | ||
| jsxLoader.invalidateAll(); | ||
| } | ||
| }); | ||
| }, | ||
| handlers(self) { | ||
| return { | ||
|
|
@@ -117,16 +143,24 @@ module.exports = { | |
| }, | ||
| 'apostrophe:destroy': { | ||
| async nunjucksLoaderCleanup() { | ||
| // Older code paths used to manage chokidar watchers per loader; | ||
| // a no-op `destroy()` is still defined for backwards compat. | ||
| for (const loader of Object.values(self.loaders || {})) { | ||
| await loader.destroy(); | ||
| } | ||
| }, | ||
| async closeViewWatchers() { | ||
| // Tear down chokidar watchers (Nunjucks + JSX share these). | ||
| await self.closeViewWatchers(); | ||
| } | ||
| } | ||
| }; | ||
| }, | ||
| methods(self) { | ||
| return { | ||
| ...require('./lib/bundlesLoader')(self), | ||
| ...require('./lib/jsxRender')(self), | ||
| ...require('./lib/viewWatcher')(self), | ||
|
|
||
| // Add helpers in the namespace for a particular module. | ||
| // They will be visible in nunjucks at | ||
|
|
@@ -261,6 +295,21 @@ module.exports = { | |
|
|
||
| let result; | ||
|
|
||
| // For named files, prefer a .jsx implementation when one exists | ||
| // anywhere in the module's view-folder chain. Falling back to | ||
| // Nunjucks happens automatically below when no JSX file is found. | ||
| if (type === 'file') { | ||
| const resolved = self.resolveTemplate(module, s); | ||
| if (resolved && resolved.kind === 'jsx') { | ||
| const renderData = self.getRenderDataArgs(req, data, module); | ||
| result = await self.renderJsxTemplate(req, resolved, renderData, module); | ||
| if (process.platform === 'win32') { | ||
| result = result.replaceAll('\r', ''); | ||
| } | ||
| return result; | ||
| } | ||
| } | ||
|
|
||
| const args = self.getRenderArgs(req, data, module); | ||
|
|
||
| const env = self.getEnv(req, module); | ||
|
|
@@ -495,6 +544,9 @@ module.exports = { | |
| } | ||
| if (!self.loaders[key]) { | ||
| self.loaders[key] = self.newLoader(moduleName, dirs); | ||
| // Register these dirs with the shared view watcher (idempotent | ||
| // per absolute path, so calling it for every loader is fine). | ||
| self.watchViewFolders(dirs); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't see anything preventing watchers in production. I might be missing something, just pointing it out. |
||
| } | ||
| return self.loaders[key]; | ||
| }, | ||
|
|
||
128 changes: 128 additions & 0 deletions
128
packages/apostrophe/modules/@apostrophecms/template/lib/jsxLoader.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| // Compiles Apostrophe `.jsx` template files via Babel and registers a | ||
| // `require.extensions['.jsx']` hook so they can be loaded with `require()` | ||
| // (and `import` after CommonJS transformation) just like normal modules. | ||
| // | ||
| // Each compiled module is automatically prefixed with a `require()` of our | ||
| // JSX runtime so the `h` and `Fragment` identifiers produced by the Babel | ||
| // transform resolve without the user importing them. Both `import` and | ||
| // `require` work inside `.jsx` files because the CommonJS transform also | ||
| // runs. | ||
| // | ||
| // Source maps are kept in memory and wired through `source-map-support`, | ||
| // which means stack traces from a JSX template point at the original | ||
| // `views/page.jsx` line/column rather than the compiled output. | ||
|
|
||
| const fs = require('fs'); | ||
| const Module = require('module'); | ||
| const babel = require('@babel/core'); | ||
| const sourceMapSupport = require('source-map-support'); | ||
|
|
||
| const runtimePath = require.resolve('./jsxRuntime.js'); | ||
|
|
||
| const sourceMaps = new Map(); | ||
| let installed = false; | ||
|
|
||
| // Idempotent: register the require hook + source-map handler once per | ||
| // process even if multiple Apostrophe instances boot in the same Node | ||
| // process (e.g. tests, multisite). | ||
| function install() { | ||
| if (installed) { | ||
| return; | ||
| } | ||
| installed = true; | ||
|
|
||
| sourceMapSupport.install({ | ||
| environment: 'node', | ||
| hookRequire: false, | ||
| handleUncaughtExceptions: false, | ||
| retrieveSourceMap(filename) { | ||
| const map = sourceMaps.get(filename); | ||
| if (!map) { | ||
| return null; | ||
| } | ||
| return { | ||
| url: filename, | ||
| map | ||
| }; | ||
| } | ||
| }); | ||
|
|
||
| Module._extensions['.jsx'] = function(module, filename) { | ||
| const src = fs.readFileSync(filename, 'utf-8'); | ||
| const compiled = compile(src, filename); | ||
| sourceMaps.set(filename, compiled.map); | ||
| module._compile(compiled.code, filename); | ||
| }; | ||
| } | ||
|
|
||
| // Compile a JSX source string for `filename`. Returns `{ code, map }`. | ||
| // `code` is CommonJS-compatible JS with our runtime injected at the top. | ||
| function compile(src, filename) { | ||
| let result; | ||
| try { | ||
| result = babel.transformSync(src, { | ||
| filename, | ||
| sourceMaps: true, | ||
| sourceFileName: filename, | ||
| babelrc: false, | ||
| configFile: false, | ||
| compact: false, | ||
| plugins: [ | ||
| [ | ||
| require.resolve('@babel/plugin-transform-react-jsx'), | ||
| { | ||
| pragma: '__aposJsx.h', | ||
| pragmaFrag: '__aposJsx.Fragment', | ||
| useBuiltIns: false, | ||
| throwIfNamespace: false | ||
| } | ||
| ], | ||
| require.resolve('@babel/plugin-transform-modules-commonjs') | ||
| ] | ||
| }); | ||
| } catch (e) { | ||
| // Babel errors already include code frames pointing at the offending | ||
| // line/column. Preserve that detail and add the file path for clarity. | ||
| const err = new Error(`JSX compile error in ${filename}: ${e.message}`); | ||
| err.cause = e; | ||
| err.code = 'APOS_JSX_COMPILE_ERROR'; | ||
| err.filename = filename; | ||
| throw err; | ||
| } | ||
|
|
||
| // Inject runtime references. Using a single `__aposJsx` namespace avoids | ||
| // colliding with user variables named `h` or `Fragment` while still | ||
| // matching the pragma we passed to Babel above. Source maps remain valid | ||
| // because we only prepend a single line and rely on a leading `\n` to | ||
| // keep line numbers stable. | ||
| const prefix = `var __aposJsx = require(${JSON.stringify(runtimePath)});\n`; | ||
| return { | ||
| code: prefix + result.code, | ||
| map: result.map | ||
| }; | ||
| } | ||
|
|
||
| // Drop a single .jsx file from the require cache and our source-map cache. | ||
| // Called by the template module's chokidar watcher when a JSX file changes, | ||
| // so the next render picks up the new code without restarting the process. | ||
| function invalidate(filename) { | ||
| sourceMaps.delete(filename); | ||
| delete Module._cache[filename]; | ||
| } | ||
|
|
||
| // Drop every cached .jsx module and source map. Used when watcher events | ||
| // don't carry a specific path or when an unknown view file was modified. | ||
| function invalidateAll() { | ||
| for (const filename of sourceMaps.keys()) { | ||
| delete Module._cache[filename]; | ||
| } | ||
| sourceMaps.clear(); | ||
| } | ||
|
|
||
| module.exports = { | ||
| install, | ||
| compile, | ||
| invalidate, | ||
| invalidateAll, | ||
| runtimePath | ||
| }; |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's odd. Shouldn't this replace be
\r\n->\n? Windows to Linux new line normalization?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\r never makes sense by itself in a source file, so it's reasonable to just dump them, knowing the \n will still be there.