Skip to content

Demos: Add Rollup and Webpack examples#1787

Merged
Krinkle merged 1 commit intoqunitjs:mainfrom
Krinkle:esm-prep-demos-rollup
Jul 30, 2024
Merged

Demos: Add Rollup and Webpack examples#1787
Krinkle merged 1 commit intoqunitjs:mainfrom
Krinkle:esm-prep-demos-rollup

Conversation

@Krinkle
Copy link
Copy Markdown
Member

@Krinkle Krinkle commented Jul 25, 2024

This is in preparation for adding an ESM distribution to QUnit. Before we do so, let's first observe how and what works today:

  • import default: import QUnit from 'qunit';
  • import named QUnit: import { QUnit } from 'qunit';
  • import named methods: import { module, test } from 'qunit';
  • require default: require('qunit')

In addition, verify that within a given bundler, a mixture of these across multple files, in indirect ways, always refers to the same object, both functionally (extensions to the object are visible to others), and by object identity (strict equality).

Specifically to satisfy Webpack, I've renamed the fixtures that use require() from .js to .cjs as Webpack refuses to compile require() calls otherwise in a directory with { type: 'module' } in package.json. Rollup does not have this restriction.

Partially based on jquery/jquery#5429 by @mgol.

Ref #1551.

@Krinkle Krinkle force-pushed the esm-prep-demos-rollup branch 4 times, most recently from 3fb2596 to 3a138e7 Compare July 29, 2024 01:28
@Krinkle Krinkle changed the title Demos: Add Rollup example with a mix of import, input, and output Demos: Add Rollup and Webpack examples Jul 29, 2024
@Krinkle Krinkle force-pushed the esm-prep-demos-rollup branch 8 times, most recently from 982e77b to 310276f Compare July 29, 2024 03:34
Copy link
Copy Markdown
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

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

I added a few questions but nothing blocking. An interesting approach with separating the bundlers test part.

dir: tmpDir,
entryFileNames: '[name].[format].js',
format: 'umd',
name: 'UNUSED'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is this about?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

When reading a file as ESM, Rollup differentiates between a file that has no exports (i.e. app code), and a file that has exports. When reading a file as CJS, Rollup doesn't understand this, and so it insists on exporting. When using umd, one of the branches assigns a global to the overall exports which requires a name. Without the build will fail.

The exports is an empty object, but Rollup wants to put it somewhere, so gave it the name window. UNUSED.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A comment in code could be useful here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ack. Done in #1791.

for await (const fileName of gRollup) {
console.log('... built ' + fileName);

if (!fileName.endsWith('.cjs.js')) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you explain this condition?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The cjs outputs (not to confuse with cjs inputs) are not meant for browser context, as they depend on module.exports and would throw an uncaught error if you try to load it there. That reminds me, I wanted to add a Node.js test case as well. Right now this file is basically unused.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you add a comment?

…re()

Co-authored-by: Michał Gołębiowski-Owczarek <m.goleb@gmail.com>
@Krinkle Krinkle force-pushed the esm-prep-demos-rollup branch from e0d6ac0 to 580a184 Compare July 30, 2024 20:03
@Krinkle Krinkle merged commit a3327c3 into qunitjs:main Jul 30, 2024
@Krinkle Krinkle deleted the esm-prep-demos-rollup branch July 30, 2024 20:36
Krinkle added a commit to Krinkle/qunit that referenced this pull request Jul 30, 2024
Krinkle added a commit that referenced this pull request Jul 30, 2024
Previously, `npm run build-dev` and `npm test` could not be run
side-by-side due to both using port 4000. I fixed this recently
in /demos/bundlers/Gruntfile.js. Let's apply the same fix here too.

Ref #1787.
Krinkle added a commit that referenced this pull request Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants