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

enhancement/issue 1218 native import attributes parsing #1321

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
3 changes: 2 additions & 1 deletion .ls-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ ls:
.woff2: lowercase

ignore:
- .git
- .git
- public
- node_modules
- packages/plugin-babel/node_modules
- packages/init/node_modules
Expand Down
9 changes: 4 additions & 5 deletions packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,12 @@
"access": "public"
},
"dependencies": {
"@rollup/plugin-commonjs": "^25.0.0",
"@rollup/plugin-commonjs": "^28.0.0",
"@rollup/plugin-node-resolve": "^15.0.0",
"@rollup/plugin-replace": "^5.0.5",
"@rollup/plugin-terser": "^0.4.4",
"acorn": "^8.0.1",
"acorn-import-attributes": "^1.9.5",
"acorn-walk": "^8.0.0",
"acorn": "^8.14.0",
"acorn-walk": "^8.3.4",
"commander": "^2.20.0",
"css-tree": "^3.0.0",
"es-module-shims": "^1.8.3",
Expand All @@ -53,7 +52,7 @@
"remark-frontmatter": "^2.0.0",
"remark-parse": "^8.0.3",
"remark-rehype": "^7.0.0",
"rollup": "^3.29.4",
"rollup": "^4.26.0",
"unified": "^9.2.0",
"wc-compiler": "~0.15.0"
},
Expand Down
23 changes: 18 additions & 5 deletions packages/cli/src/config/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,19 @@

return {
name: 'greenwood-resource-loader',
async resolveId(id, importer) {
async resolveId(id, importer, options) {
const normalizedId = cleanRollupId(id);
const { userWorkspace } = compilation.context;

// check for non bare paths and resolve them to the user's workspace
// or Greenwood's scratch dir, like when bundling inline <script> tags
if (normalizedId.startsWith('.')) {
const importerUrl = new URL(normalizedId, `file://${importer}`);
const extension = importerUrl.pathname.split('.').pop();
const type = options.attributes?.type ?? '';
// if we are polyfilling import attributes for the browser we will want Rollup to bundles these as JS files
// instead of externalizing as their native content-type
const shouldPolyfill = browser && (importAttributes || []).includes(extension);
const external = !shouldPolyfill && externalizedResources.includes(extension) && browser && !importerUrl.searchParams.has('type');
const shouldPolyfill = browser && (importAttributes || []).includes(type);
const external = !shouldPolyfill && externalizedResources.includes(type) && browser && !importerUrl.searchParams.has('type');
const isUserWorkspaceUrl = importerUrl.pathname.startsWith(userWorkspace.pathname);
const prefix = normalizedId.startsWith('..') ? './' : '';
// if its not in the users workspace, we clean up the dot-dots and check that against the user's workspace
Expand Down Expand Up @@ -267,7 +267,7 @@
let canTransform = false;
let response = new Response(code);

// handle any custom imports or pre-processing needed before passing to Rollup this.parse
// handle any custom imports or pre-processing first to ensure valid JavaScript for parsing
if (await checkResourceExists(idUrl)) {
for (const plugin of resourcePlugins) {
if (plugin.shouldResolve && await plugin.shouldResolve(idUrl)) {
Expand Down Expand Up @@ -301,6 +301,9 @@
return null;
}

// ideally we would use our own custom Acorn config + parsing
// but need to wait for Rollup to remove `assert` which will break Acorn, which only understands `with`
// https://github.com/rollup/rollup/issues/5685
const ast = this.parse(await response.text());
const assetUrls = [];
let modifiedCode = false;
Expand Down Expand Up @@ -369,7 +372,7 @@
}
}
} else {
// TODO figure out how to handle URL chunk from SSR pages

Check warning on line 375 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO figure out how to handle URL chunk...'

Check warning on line 375 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO figure out how to handle URL chunk...'

Check warning on line 375 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected 'todo' comment: 'TODO figure out how to handle URL chunk...'

Check warning on line 375 in packages/cli/src/config/rollup.config.js

View workflow job for this annotation

GitHub Actions / build (18)

Unexpected ' TODO' comment: 'TODO figure out how to handle URL chunk...'
// https://github.com/ProjectEvergreen/greenwood/issues/1163
}

Expand Down Expand Up @@ -444,6 +447,14 @@
}

const { code } = bundles[bundle];

if (!code) {
return;
}

// ideally we would use our own custom Acorn config + parsing
// but need to wait for Rollup to remove `assert` which will break Acorn, which only understands `with`
// https://github.com/rollup/rollup/issues/5685
const ast = this.parse(code);

walk.simple(ast, {
Expand All @@ -465,6 +476,8 @@
}
});
} else {
// waiting on Rollup to formally support `with`
// https://github.com/rollup/rollup/issues/5685
bundles[bundle].code = bundles[bundle].code.replace(/assert{/g, 'with{');
}

Expand Down
8 changes: 8 additions & 0 deletions packages/cli/src/lib/parsing-utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const acornOptions = {
ecmaVersion: 'latest',
sourceType: 'module'
};

export {
acornOptions
};
6 changes: 2 additions & 4 deletions packages/cli/src/lib/walker-package-ranger.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import fs from 'fs';
import { getNodeModulesLocationForPackage } from './node-modules-utils.js';
import path from 'path';
import * as walk from 'acorn-walk';
import { acornOptions } from './parsing-utils.js';

const importMap = {};

Expand Down Expand Up @@ -46,10 +47,7 @@ async function getPackageEntryPath(packageJson) {
async function walkModule(modulePath, dependency) {
const moduleContents = fs.readFileSync(modulePath, 'utf-8');

walk.simple(acorn.parse(moduleContents, {
ecmaVersion: '2020',
sourceType: 'module'
}), {
walk.simple(acorn.parse(moduleContents, acornOptions), {
async ImportDeclaration(node) {
let { value: sourceValue } = node.source;
const absoluteNodeModulesLocation = await getNodeModulesLocationForPackage(dependency);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { ResourceInterface } from '../../lib/resource-interface.js';
import terser from '@rollup/plugin-terser';
import * as acorn from 'acorn';
import * as walk from 'acorn-walk';
import { importAttributes } from 'acorn-import-attributes';
import { acornOptions } from '../../lib/parsing-utils.js';

class StandardJavaScriptResource extends ResourceInterface {
constructor(compilation, options) {
Expand Down Expand Up @@ -43,10 +43,7 @@ class StandardJavaScriptResource extends ResourceInterface {
const body = await response.clone().text();
let polyfilled = body;

walk.simple(acorn.Parser.extend(importAttributes).parse(body, {
ecmaVersion: 'latest',
sourceType: 'module'
}), {
walk.simple(acorn.parse(body, acornOptions), {
async ImportDeclaration(node) {
const line = body.slice(node.start, node.end);
const { value } = node.source;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ describe('Build Greenwood With: ', function() {
const inlineScriptTag = Array.from(dom.window.document.querySelectorAll('head > script:not([src])')).filter(tag => !tag.getAttribute('data-gwd'))[0];

expect(inlineScriptTag.textContent.replace(/\n/g, '')).to
.equal('import"/116321042.4f3171e3.js";import"/lit-html.31ea57aa.js";//# sourceMappingURL=116321042.69f46fc1.js.map');
.equal('import"/116321042.dlaVsmnb.js";import"/lit-html.CYd3Xodq.js";//# sourceMappingURL=116321042.SNvCd9wk.js.map');
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,19 @@ describe('Build Greenwood With: ', function() {
it('should have one <script> tag for other.js loaded in the <head>', function() {
const scriptTags = dom.window.document.querySelectorAll('head > script[type="module"]');
const mainScriptTags = Array.prototype.slice.call(scriptTags).filter(script => {
return (/other.*[a-z0-9].js/).test(script.src);
return (/other.*[a-zA-Z0-9].js/).test(script.src);
});

expect(mainScriptTags.length).to.be.equal(1);
});

// this includes the non module file in a spec below
it('should have the expected number of bundled .js files in the output directory', async function() {
expect(await glob.promise(path.join(this.context.publicDir, '*.*[a-z0-9].js'))).to.have.lengthOf(3);
expect(await glob.promise(path.join(this.context.publicDir, '*.*[a-zA-Z0-9].js'))).to.have.lengthOf(3);
});

it('should have the expected other.js file in the output directory', async function() {
expect(await glob.promise(path.join(this.context.publicDir, 'other.*[a-z0-9].js'))).to.have.lengthOf(1);
expect(await glob.promise(path.join(this.context.publicDir, 'other.*[a-zA-Z0-9].js'))).to.have.lengthOf(1);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('Build Greenwood With: ', function() {
});

describe('Custom Element Importing CSS w/ Constructable Stylesheet', function() {
const cssFileHash = 'bcdce3a3';
const cssFileHash = 'YPhf6BPs';
let scripts;
let styles;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('Build Greenwood With: ', function() {
runSmokeTest(['public'], LABEL);

describe('Custom Element Importing CSS w/ Constructable Stylesheet', function() {
const cssFileHash = '93e8cf36';
const cssFileHash = 'Cg2xlj-Z';
let scripts;
let styles;

Expand All @@ -70,7 +70,7 @@ describe('Build Greenwood With: ', function() {
const scriptContents = fs.readFileSync(scripts[0], 'utf-8');

expect(scripts.length).to.equal(1);
expect(scriptContents).to.contain('import e from"/hero.93e8cf36.css"with{type:"css"}');
expect(scriptContents).to.contain('import e from"/hero.Cg2xlj-Z.css"with{type:"css"}');
});

it('should have the expected CSS output bundle for hero.css', function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ describe('Serve Greenwood With: ', function() {
});

describe('Import Attributes Polyfill Behaviors for the initiating JavaScript file (hero.js) being served and bundled', function() {
const jsHash = '9acf7b4c';
const jsHash = 'B7j93uYq';
let response = {};
let text;
let contents;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe('Serve Greenwood With: ', function() {
const publicPath = path.join(outputPath, 'public/');
const hostname = 'http://127.0.0.1:8080';
const basePath = '/my-path';
const jsHash = '2ce3f02d';
const jsHash = 'eCbOYUm_';
const cssHash = '2106293974';
let runner;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ describe('Serve Greenwood With: ', function() {
});

describe('Bundled image using new URL and import.meta.url', function() {
const bundledName = 'assets/logo-619de195.svg';
const bundledName = 'assets/logo-DQvO5Aou.svg';
let response = {};
let body;
let usersResponse = {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,33 @@ template.innerHTML = `
`;

class MyCounter extends HTMLElement {
static staticProperty = 'foo';
#count;

constructor() {
super();
this.count = 0;
this.#count = 0;
this.attachShadow({ mode: 'open' });
}

async connectedCallback() {
this.shadowRoot.appendChild(template.content.cloneNode(true));
this.shadowRoot.getElementById('inc').onclick = () => this.inc();
this.shadowRoot.getElementById('dec').onclick = () => this.dec();
this.#count = this.getAttribute('count') ?? this.#count;
this.update();
}

inc() {
this.update(++this.count); // eslint-disable-line
this.update(++this.#count); // eslint-disable-line
}

dec() {
this.update(--this.count); // eslint-disable-line
this.update(--this.#count); // eslint-disable-line
}

update(count) {
this.shadowRoot.getElementById('count').innerHTML = count || this.count;
this.shadowRoot.getElementById('count').innerHTML = count || this.#count;
}
}

Expand Down
5 changes: 2 additions & 3 deletions packages/plugin-css-modules/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@
"@greenwood/cli": "^0.30.0"
},
"dependencies": {
"acorn": "^8.0.1",
"acorn-import-attributes": "^1.9.5",
"acorn-walk": "^8.0.0",
"acorn": "^8.14.0",
"acorn-walk": "^8.3.4",
"css-tree": "^3.0.0",
"node-html-parser": "^1.2.21",
"sucrase": "^3.35.0"
Expand Down
14 changes: 3 additions & 11 deletions packages/plugin-css-modules/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import { ResourceInterface } from '@greenwood/cli/src/lib/resource-interface.js'
import * as acornWalk from 'acorn-walk';
import * as acorn from 'acorn';
import { hashString } from '@greenwood/cli/src/lib/hashing-utils.js';
import { importAttributes } from 'acorn-import-attributes';
import { transform } from 'sucrase';
import { acornOptions } from '@greenwood/cli/src/lib/parsing-utils.js';

const MODULES_MAP_FILENAME = '__css-modules-map.json';
/*
Expand Down Expand Up @@ -40,11 +40,7 @@ function walkAllImportsForCssModules(scriptUrl, sheets, compilation) {
});

acornWalk.simple(
acorn.Parser.extend(importAttributes).parse(result.code, {
ecmaVersion: 'latest',
sourceType: 'module'
}),
{
acorn.parse(result.code, acornOptions), {
ImportDeclaration(node) {
const { specifiers = [], source = {} } = node;
const { value = '' } = source;
Expand Down Expand Up @@ -246,11 +242,7 @@ class StripCssModulesResource extends ResourceInterface {
let contents = await response.text();

acornWalk.simple(
acorn.Parser.extend(importAttributes).parse(contents, {
ecmaVersion: 'latest',
sourceType: 'module'
}),
{
acorn.parse(contents, acornOptions), {
ImportDeclaration(node) {
const { specifiers = [], source = {}, start, end } = node;
const { value = '' } = source;
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-import-commonjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"@greenwood/cli": "^0.30.0"
},
"dependencies": {
"@rollup/plugin-commonjs": "^25.0.0",
"@rollup/plugin-commonjs": "^28.0.0",
"@rollup/stream": "^3.0.1",
"cjs-module-lexer": "^1.0.0"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@ describe('Build Greenwood With: ', function() {
it('should have the expected CommonJS contents from main.js (lodash) in the output', async function() {
const contents = fs.readFileSync(scripts[0], 'utf-8');

expect(contents).to.contain('r=u,e=u.exports,function()');
expect(contents).to.contain('document.getElementsByTagName("span")[0].innerHTML=`import from lodash ${o}`');
expect(contents).to.contain('document.getElementsByTagName("span")[0].innerHTML=`import from lodash ${a}`');
});
});
});
Expand Down
Loading
Loading