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

Circular dependency in builds: lib/calendar.mjs -> lib/ecmascript.mjs -> lib/calendar.mjs #1008

Closed
justingrant opened this issue Oct 16, 2020 · 7 comments
Labels
bug non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE!
Milestone

Comments

@justingrant
Copy link
Collaborator

A circular dependency has crept into the builds recently.

justingrant@jgmac proposal-temporal % npm run build

> proposal-temporal@1.0.0 prebuild /Users/justingrant/Documents/hdev/proposal-temporal
> mkdirp out/docs/assets


> proposal-temporal@1.0.0 build /Users/justingrant/Documents/hdev/proposal-temporal
> npm run build:polyfill && npm run build:docs && npm run build:spec


> proposal-temporal@1.0.0 build:polyfill /Users/justingrant/Documents/hdev/proposal-temporal
> cd polyfill && npm install && npm run build

audited 459 packages in 2.066s

29 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities


> proposal-temporal@0.4.0 build /Users/justingrant/Documents/hdev/proposal-temporal/polyfill
> rollup -c rollup.config.js


lib/index.mjs → ./dist/index.js, ./dist/index.umd.js...
(!) Circular dependency
lib/calendar.mjs -> lib/ecmascript.mjs -> lib/calendar.mjs
created ./dist/index.js, ./dist/index.umd.js in 3.6s

lib/shim.mjs → script.js...
(!) Circular dependency
lib/calendar.mjs -> lib/ecmascript.mjs -> lib/calendar.mjs
created script.js in 1.3s

lib/shim.mjs → ../out/docs/playground.js...
(!) Circular dependency
lib/calendar.mjs -> lib/ecmascript.mjs -> lib/calendar.mjs
created ../out/docs/playground.js in 2.7s

> proposal-temporal@1.0.0 build:docs /Users/justingrant/Documents/hdev/proposal-temporal
> cd docs && npm install && npm run build

audited 8 packages in 0.289s
found 0 vulnerabilities


> tc39-temporal-playground@1.0.0 build /Users/justingrant/Documents/hdev/proposal-temporal/docs
> node buildDocs.js

README.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/README.html
calendar.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/calendar.html
balancing.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/balancing.html
localdatetime-draft.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/localdatetime-draft.html
now.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/now.html
calendar-subclass.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/calendar-subclass.html
timezone-draft.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/timezone-draft.html
iso-string-ext.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/iso-string-ext.html
ambiguity.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/ambiguity.html
calendar-draft.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/calendar-draft.html
monthday.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/monthday.html
duration.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/duration.html
date.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/date.html
instant.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/instant.html
parse-draft.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/parse-draft.html
time.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/time.html
datetime.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/datetime.html
yearmonth.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/yearmonth.html
timezone.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/timezone.html
cookbook.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/cookbook.html
zoneddatetime.md => /Users/justingrant/Documents/hdev/proposal-temporal/out/docs/zoneddatetime.html

> proposal-temporal@1.0.0 build:spec /Users/justingrant/Documents/hdev/proposal-temporal
> ecmarkup --lint-spec --strict spec.html out/index.html

``
@ptomato ptomato added bug non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! labels Oct 16, 2020
@ptomato ptomato added this to the Stage 3 milestone Oct 16, 2020
@Ms2ger
Copy link
Collaborator

Ms2ger commented Oct 19, 2020

It's

import { GetISO8601Calendar } from './calendar.mjs';

Should use GetIntrinsic('%Temporal.ISO8601Calendar%') instead.

@justingrant
Copy link
Collaborator Author

Is Temporal.ISO8601Calendar a public API? I don't remember discussing it.

@ptomato
Copy link
Collaborator

ptomato commented Oct 19, 2020

It's discussed in #847

@justingrant
Copy link
Collaborator Author

Sorry, I was asking a more specific question: how (if at all) will those built-in calendars be exposed on the Temporal namespace? In my IDE, will I see ISO8601Calendar as a peer of Instant?

@ptomato
Copy link
Collaborator

ptomato commented Oct 19, 2020

Currently, yes, although that might change if we get reviewer feedback on #847.

@justingrant
Copy link
Collaborator Author

I strongly prefer Temporal.Calendar.Japanese over Temporal.JapaneseCalendar for IDE usability reasons. Let's continue discussion over in #847.

@ptomato
Copy link
Collaborator

ptomato commented Nov 16, 2020

Fixed by #1195.

@ptomato ptomato closed this as completed Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE!
Projects
None yet
Development

No branches or pull requests

3 participants